Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[wip] Revise the raster cache unit test #17790

Closed
wants to merge 1 commit into from
Closed

[wip] Revise the raster cache unit test #17790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions flow/raster_cache.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,8 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const {
SkAutoCanvasRestore auto_restore(&canvas, true);
SkIRect bounds =
RasterCache::GetDeviceBounds(logical_rect_, canvas.getTotalMatrix());
FML_DCHECK(
std::abs(bounds.size().width() - image_->dimensions().width()) <= 1 &&
std::abs(bounds.size().height() - image_->dimensions().height()) <= 1);
FML_DCHECK(bounds.size().width() == image_->dimensions().width() &&
bounds.size().height() == image_->dimensions().height());
canvas.resetMatrix();
canvas.drawImage(image_, bounds.fLeft, bounds.fTop, paint);
}
Expand Down
6 changes: 5 additions & 1 deletion flow/raster_cache_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,11 @@ TEST(RasterCache, DeviceRectRoundOut) {
canvas.setMatrix(ctm);
canvas.translate(248, 0);

cache.Get(*picture, ctm).draw(canvas);
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
canvas.setMatrix(RasterCache::GetIntegralTransCTM(canvas.getTotalMatrix()));
Copy link
Contributor

@flar flar Apr 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line defeats the purpose of this test as stated in the comment above it.

Copy link
Contributor Author

@liyuqian liyuqian Apr 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't snap, we'd fail the integral translation assert instead of of bounds assert above. This test tries to reproduce the bounds assert.

Copy link
Contributor

@flar flar Apr 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the current way that we use the raster cache, this whole test doesn't make sense then.

It will be useful when we introduce "cache at 0,0 - render at any fractional translation" later, though. In that case we will see output size differences of up to +1 from the cached image.

#endif

cache.Get(*picture, canvas.getTotalMatrix()).draw(canvas);
}

} // namespace testing
Expand Down