Summary: | When QD plugins draw to an offscreen bitmap the origin is not correct | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Bruce Q. Hammond <bruceq> | ||||||||
Component: | Plug-ins | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | Keywords: | InRadar | ||||||||
Priority: | P2 | ||||||||||
Version: | 523.x (Safari 3) | ||||||||||
Hardware: | Mac | ||||||||||
OS: | OS X 10.4 | ||||||||||
Attachments: |
|
Description
Bruce Q. Hammond
2007-05-03 16:55:35 PDT
Created attachment 14323 [details]
QD plugins now respect the origin of CGBitmapContext
Comment on attachment 14323 [details]
QD plugins now respect the origin of CGBitmapContext
Looks fine. But why not use CGPointApplyAffineTransform instead of doing explicit multiplication?
Comment on attachment 14323 [details]
QD plugins now respect the origin of CGBitmapContext
(int32)-boundsInWindow.origin.y - (offscreenBounds.bottom + offscreenMatrix.ty * offscreenMatrix.d)
Please explain why you need to multiply offscreenMatrix.ty by offscreenMatrix.d but not do something similar for offscreenMatrix.tx. Please explain this code better in the comments.
Can you also use CGPointApplyAffineTransform like Darin asked?
Comment on attachment 14323 [details]
QD plugins now respect the origin of CGBitmapContext
Someone told me that Bruce is working on a version of this that uses CGPointApplyAffineTransform, so review- until we have that one.
Created attachment 14404 [details]
respect the origin implied by the bitmap contexts affine tranformation matrix
Implements the changes requested by reviewers to go though CGPointApplyAffineTransform to create the origin data. Personally I don't like this patch as much as the previous one because it requires yet another call to CGPointApplyAffineTransform in order to determine if the axes are flipped. However it is simply a mater of taste and is functionally identical; it may be more clear to some maintainers what is happening.
I also now treat Y-Axis and and X-Axis identically even though inverting the X-Axis would be unusual.
Comment on attachment 14404 [details]
respect the origin implied by the bitmap contexts affine tranformation matrix
r=me, assuming that you (Bruce) have a test case that shows this works well, since Safari doesn't use this code path.
(In reply to comment #7) > r=me, assuming that you (Bruce) have a test case that shows this works well, > since Safari doesn't use this code path. Bruce confirmed that he does have a test, so confirmed: r=me. A build fix in r21323 was needed to keep things building with the ancient version of Xcode the buildslaves are currently using. Created attachment 14573 [details]
Fixes the sign of the Y-Origin adjustment
The Y-origin adjustment was backwards on the previous patch. This fixes it.
Comment on attachment 14573 [details]
Fixes the sign of the Y-Origin adjustment
It'd be *really* handy if all of these changes had some form of regression testing. There have been several issues with past iterations of related patches, and without any way to test these (Safari doesn't touch this code path) it's impossible to know if and when this codepath breaks in the future.
|