RESOLVED FIXED 26749
Canvas: rotation of 'no-repeat' pattern still has small error
https://bugs.webkit.org/show_bug.cgi?id=26749
Summary Canvas: rotation of 'no-repeat' pattern still has small error
Shinichiro Hamaji
Reported 2009-06-25 21:54:37 PDT
I didn't put expected image into my patch in https://bugs.webkit.org/show_bug.cgi?id=26426 Also, it would be better to use (1<<23)/2 as steps to make the error less than 0.5. I could see the 1 pixel errors with the previous value.
Attachments
Patch v1 (153.24 KB, patch)
2009-06-25 21:56 PDT, Shinichiro Hamaji
oliver: review-
an HTML to describe this bug (1.40 KB, text/html)
2009-07-16 04:12 PDT, Shinichiro Hamaji
no flags
Before the patch (1.20 KB, image/png)
2009-07-16 04:13 PDT, Shinichiro Hamaji
no flags
After the patch (1.19 KB, image/png)
2009-07-16 04:14 PDT, Shinichiro Hamaji
no flags
Patch v2 (167.00 KB, patch)
2009-07-16 04:47 PDT, Shinichiro Hamaji
oliver: review+
platform/mac/fast/canvas/image-object-in-canvas-expected.png (76.86 KB, image/png)
2009-07-24 07:58 PDT, Shinichiro Hamaji
no flags
platform/mac/fast/canvas/image-pattern-rotate-expected.png (16.07 KB, image/png)
2009-07-24 08:00 PDT, Shinichiro Hamaji
no flags
Shinichiro Hamaji
Comment 1 2009-06-25 21:56:48 PDT
Created attachment 31904 [details] Patch v1 LayoutTests/ChangeLog | 15 +++++++++++++++ .../image-object-in-canvas-expected.checksum | 2 +- .../canvas/image-object-in-canvas-expected.png | Bin 56436 -> 78707 bytes WebCore/ChangeLog | 15 +++++++++++++++ WebCore/platform/graphics/cg/PatternCG.cpp | 7 +++---- 5 files changed, 34 insertions(+), 5 deletions(-)
Shinichiro Hamaji
Comment 2 2009-06-25 21:58:25 PDT
Adding Oliver into CC list as he reviewed my previous patch.
Oliver Hunt
Comment 3 2009-07-15 23:51:11 PDT
Comment on attachment 31904 [details] Patch v1 I'm not sure what you're trying to fix here. If this patch is correct you should just be doing 1<<22, but like i said i'm not sure what you're actually trying to do.
Shinichiro Hamaji
Comment 4 2009-07-16 04:12:27 PDT
Created attachment 32853 [details] an HTML to describe this bug
Shinichiro Hamaji
Comment 5 2009-07-16 04:13:43 PDT
Created attachment 32854 [details] Before the patch
Shinichiro Hamaji
Comment 6 2009-07-16 04:14:19 PDT
Created attachment 32855 [details] After the patch
Shinichiro Hamaji
Comment 7 2009-07-16 04:27:22 PDT
Sorry for the poor description/title of this bug. I've attached an HTML which can describe why I want to use smaller value than before. 1<<23 is the precision of mantissa of floats and using this means that the error is <1px. I think 1px error can be still visible. The HTML I've attached is putting 3x3 image whose shape is '+' vertically. As it doesn't change the value of X-axis, you should see a vertical straight line like "After the patch". However, with current WebKit, the line is not straight like "Before the patch". If we use 1<<22 as this value, the error should be <0.5px. As WebKit is doing anti-alias for rotation, the image changes if we use 1<<21 or something smaller value. However, I think no one would be care about such tiny changes. So, I think 1<<22 would be OK. I'll update the patch with more description and test case.
Shinichiro Hamaji
Comment 8 2009-07-16 04:47:13 PDT
Created attachment 32859 [details] Patch v2
Oliver Hunt
Comment 9 2009-07-23 19:51:33 PDT
Comment on attachment 32859 [details] Patch v2 ChangeLog needs to be updated to say 1<<22 not (1<<23)/2 but this is otherwise okay -- can the person who lands fix up the changelog?
Adam Barth
Comment 10 2009-07-24 01:16:15 PDT
(In reply to comment #9) > (From update of attachment 32859 [details]) > ChangeLog needs to be updated to say 1<<22 not (1<<23)/2 but this is otherwise > okay -- can the person who lands fix up the changelog? Ok. I'll do the fixup.
Adam Barth
Comment 11 2009-07-24 01:19:16 PDT
Sorry, bugzilla-tool isn't awesome enough to handle this PNG.
Shinichiro Hamaji
Comment 12 2009-07-24 07:58:20 PDT
Created attachment 33446 [details] platform/mac/fast/canvas/image-object-in-canvas-expected.png
Shinichiro Hamaji
Comment 13 2009-07-24 08:00:05 PDT
Created attachment 33447 [details] platform/mac/fast/canvas/image-pattern-rotate-expected.png
Shinichiro Hamaji
Comment 14 2009-07-24 08:01:24 PDT
Thanks Oliver for your review, and thanks Adam for trying to fix the ChangeLog and land this patch. I've uploaded png images in this patch. Please use them if you need.
Adam Barth
Comment 15 2009-07-25 14:36:55 PDT
Trying again.
Adam Barth
Comment 16 2009-07-25 14:44:14 PDT
Wow, that was complicated. Bugzilla-tool validating patch. Hopefully will be landed shortly.
Adam Barth
Comment 17 2009-07-25 15:01:00 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog A LayoutTests/fast/canvas/image-pattern-rotate.html M LayoutTests/platform/mac/fast/canvas/image-object-in-canvas-expected.checksum M LayoutTests/platform/mac/fast/canvas/image-object-in-canvas-expected.png A LayoutTests/platform/mac/fast/canvas/image-pattern-rotate-expected.checksum A LayoutTests/platform/mac/fast/canvas/image-pattern-rotate-expected.png A LayoutTests/platform/mac/fast/canvas/image-pattern-rotate-expected.txt M WebCore/ChangeLog M WebCore/platform/graphics/cg/PatternCG.cpp Committed r46399 M WebCore/ChangeLog M WebCore/platform/graphics/cg/PatternCG.cpp A LayoutTests/platform/mac/fast/canvas/image-pattern-rotate-expected.txt M LayoutTests/platform/mac/fast/canvas/image-object-in-canvas-expected.png A LayoutTests/platform/mac/fast/canvas/image-pattern-rotate-expected.checksum A LayoutTests/platform/mac/fast/canvas/image-pattern-rotate-expected.png M LayoutTests/platform/mac/fast/canvas/image-object-in-canvas-expected.checksum M LayoutTests/ChangeLog A LayoutTests/fast/canvas/image-pattern-rotate.html r46399 = 8574a2ebe1893f5ee6b2b1fb26e1b18debbdf507 (trunk) No changes between current HEAD and refs/remotes/trunk Resetting to the latest refs/remotes/trunk http://trac.webkit.org/changeset/46399
Adam Barth
Comment 18 2009-07-25 15:12:00 PDT
Forgot to fixup the changelog before landing, but I did in the next commit: Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog M WebCore/ChangeLog Committed r46400
Note You need to log in before you can comment on or make changes to this bug.