Summary: | [Qt] REGRESSION(r) Two pixel result fail after r153522 | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zoltan Arvai <zarvai> | ||||||||||||||
Component: | Tools / Tests | Assignee: | Allan Sandfeld Jensen <allan.jensen> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | abrhm, allan.jensen, commit-queue, hausmann, jturcotte, kadam, noam, vsavchuk | ||||||||||||||
Priority: | P2 | Keywords: | Qt | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 79666, 119263 | ||||||||||||||||
Attachments: |
|
Description
Zoltan Arvai
2013-08-01 05:42:50 PDT
Created attachment 207916 [details]
image-rescale-actual
Created attachment 207917 [details]
image-small-width-height-actual
Yes, I am assuming this is caused by a QPainter with an upscale transform applied. In this case we end up downscaling and then upscaling again. Created attachment 208190 [details]
Patch
Created attachment 208196 [details]
Patch
Fix logic in checking for 2x downscale, which gave up too easy.
Comment on attachment 208196 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=208196&action=review lgtm, some nitpicks but r=me anyway if you want to land directly. > Source/WebCore/platform/graphics/qt/ImageQt.cpp:247 > + if (transform.type() > QTransform::TxScale) Nit: This breaks QTransform's abstraction by assuming details from the implementation and requires opening qtransform.h to understand. Straight & and | on the unsupported flags would be clearer. > Source/WebCore/platform/graphics/qt/ImageQt.cpp:303 > + image = prescaleImageIfRequired(ctxt->platformContext(), image, &prescaledBuffer, normalizedDst, normalizedSrc); Nit: Passing normalizedSrc as a QRectF* would make it clear from the call site that there might be side effects on it. (In reply to comment #6) > (From update of attachment 208196 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=208196&action=review > > lgtm, some nitpicks but r=me anyway if you want to land directly. > > > Source/WebCore/platform/graphics/qt/ImageQt.cpp:247 > > + if (transform.type() > QTransform::TxScale) > > Nit: This breaks QTransform's abstraction by assuming details from the implementation and requires opening qtransform.h to understand. Straight & and | on the unsupported flags would be clearer. > That is how QTransform::type() is documented. It is not a set of flags, it just returns the highest included transform. > > Source/WebCore/platform/graphics/qt/ImageQt.cpp:303 > > + image = prescaleImageIfRequired(ctxt->platformContext(), image, &prescaledBuffer, normalizedDst, normalizedSrc); > > Nit: Passing normalizedSrc as a QRectF* would make it clear from the call site that there might be side effects on it. Okay. I can change that. Created attachment 208274 [details]
Patch
Committed r153790: <http://trac.webkit.org/changeset/153790> Comment on attachment 208274 [details]
Patch
Removing the flag (I guess you didn't want r? here)
|