Bug 26749 - Canvas: rotation of 'no-repeat' pattern still has small error
Summary: Canvas: rotation of 'no-repeat' pattern still has small error
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-25 21:54 PDT by Shinichiro Hamaji
Modified: 2009-07-25 15:12 PDT (History)
3 users (show)

See Also:


Attachments
Patch v1 (153.24 KB, patch)
2009-06-25 21:56 PDT, Shinichiro Hamaji
oliver: review-
Details | Formatted Diff | Diff
an HTML to describe this bug (1.40 KB, text/html)
2009-07-16 04:12 PDT, Shinichiro Hamaji
no flags Details
Before the patch (1.20 KB, image/png)
2009-07-16 04:13 PDT, Shinichiro Hamaji
no flags Details
After the patch (1.19 KB, image/png)
2009-07-16 04:14 PDT, Shinichiro Hamaji
no flags Details
Patch v2 (167.00 KB, patch)
2009-07-16 04:47 PDT, Shinichiro Hamaji
oliver: review+
Details | Formatted Diff | Diff
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 Details
platform/mac/fast/canvas/image-pattern-rotate-expected.png (16.07 KB, image/png)
2009-07-24 08:00 PDT, Shinichiro Hamaji
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Shinichiro Hamaji 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.
Comment 1 Shinichiro Hamaji 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(-)
Comment 2 Shinichiro Hamaji 2009-06-25 21:58:25 PDT
Adding Oliver into CC list as he reviewed my previous patch.
Comment 3 Oliver Hunt 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.
Comment 4 Shinichiro Hamaji 2009-07-16 04:12:27 PDT
Created attachment 32853 [details]
an HTML to describe this bug
Comment 5 Shinichiro Hamaji 2009-07-16 04:13:43 PDT
Created attachment 32854 [details]
Before the patch
Comment 6 Shinichiro Hamaji 2009-07-16 04:14:19 PDT
Created attachment 32855 [details]
After the patch
Comment 7 Shinichiro Hamaji 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.
Comment 8 Shinichiro Hamaji 2009-07-16 04:47:13 PDT
Created attachment 32859 [details]
Patch v2
Comment 9 Oliver Hunt 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?
Comment 10 Adam Barth 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.
Comment 11 Adam Barth 2009-07-24 01:19:16 PDT
Sorry, bugzilla-tool isn't awesome enough to handle this PNG.
Comment 12 Shinichiro Hamaji 2009-07-24 07:58:20 PDT
Created attachment 33446 [details]
platform/mac/fast/canvas/image-object-in-canvas-expected.png
Comment 13 Shinichiro Hamaji 2009-07-24 08:00:05 PDT
Created attachment 33447 [details]
platform/mac/fast/canvas/image-pattern-rotate-expected.png
Comment 14 Shinichiro Hamaji 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.
Comment 15 Adam Barth 2009-07-25 14:36:55 PDT
Trying again.
Comment 16 Adam Barth 2009-07-25 14:44:14 PDT
Wow, that was complicated.  Bugzilla-tool validating patch.  Hopefully will be landed shortly.
Comment 17 Adam Barth 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
Comment 18 Adam Barth 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