Summary: | Fix a bug in pattern transformation in PatternSkia | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hin-Chung Lam <hclam> | ||||||||||||
Component: | Platform | Assignee: | Stephen White <senorblanco> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | brettw, commit-queue, dglazkov, krit, senorblanco, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | PC | ||||||||||||||
OS: | OS X 10.5 | ||||||||||||||
Attachments: |
|
Description
Hin-Chung Lam
2009-03-11 18:58:52 PDT
Created attachment 28509 [details]
Patch to patternSkia.cpp
Created attachment 28511 [details]
patch + change log
Brett, can you look at this change? I would like to let the Skia folks know about this before we work around something. If it is a bug, it should be fixed there, or we should know a fix is pending before checking in. I'll forward it. (In reply to comment #4) > I would like to let the Skia folks know about this before we work around > something. If it is a bug, it should be fixed there, or we should know a fix is > pending before checking in. I'll forward it. > Any updates from skia people? There is maybe another way to support transformations for patterns. On CG and Qt we transform the context right before we stroke/fill anything with a pattern, the path should not be affected. Just look at fillPath/strokePath in GraphicsContextCg.cpp or GraphicsContextQt.cpp to know what I mean. Could that avoid this hack at the end of the pattern-patch? To round and cut off some values seems to be incorrect for me. Comment on attachment 28511 [details]
patch + change log
r- based on Dirk's comments. Mostly I can't quite tell if this is right, Dirk is questioning the correctness, and we're trying to clear out the review queue one. Feel free to mark this r=? with answers to Dirk's questions.
No progress on this bug? :-( Created attachment 47643 [details]
New fix
The good news is, the Skia bug should be fixed, so the hack part of Alpha's fix (above) should no longer be necessary.
The bad news is, I changed the allocation of SkShaders a few months back to be done once on first draw, so it was necessary to rework Patterns a bit so that the m_patternSpaceTransform could be forwarded to the SkShader.
Created attachment 47644 [details]
ChangeLog and non-Skia fixes
Attachment 47644 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/217145 Created attachment 47652 [details]
Same as above, plus QT fix
Comment on attachment 47652 [details]
Same as above, plus QT fix
I would have probably put the "else" into its own function, but OK.
Comment on attachment 47652 [details] Same as above, plus QT fix Clearing flags on attachment: 47652 Committed r54203: <http://trac.webkit.org/changeset/54203> All reviewed patches have been landed. Closing bug. |