Summary: | [Qt] When a layer is really opaque (ie has a background color that has 255 alpha) we should fill the layer with that color to tell Qt that the pixemap has no alpha. | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sam Magnuson <smagnuso> | ||||||||||||||
Component: | WebKit Qt | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
Status: | RESOLVED DUPLICATE | ||||||||||||||||
Severity: | Normal | CC: | commit-queue, kenneth, noam, simon.fraser | ||||||||||||||
Priority: | P2 | Keywords: | Qt, QtTriaged | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | PC | ||||||||||||||||
OS: | OS X 10.5 | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 38744 | ||||||||||||||||
Attachments: |
|
Description
Sam Magnuson
2010-06-09 11:47:49 PDT
Created attachment 58276 [details]
Adding support for setBackgroundColor/clearBackground color and using it in QGraphicsLayer to pre-fill the pixmap cache.
This also means that we have to recache when the background-color changes; I think that's missing from the patch :) I agree with you in principal - I can certainly toss out the cache if you'd like. This sort of goes back to our discussion that I get setNeedsDisplay later anyway (just as we do for setContentsToImage, where we don't toss out the cache and we just get a setNeedsDisplay later). Do you want me to go ahead and do it for this case, or should I look into that bug as a whole class of bugs later? As I mentioned to you before I'd like to fix the setNeedsDisplay that we get, since when some things change (like zorder, opacity, etc) we don't need a setNeedsDisplay from WebKit as it causes us to throw away a perfectly valid cached pixmap. Created attachment 58558 [details]
Proposed patch with proper coding style
Comment on attachment 58558 [details]
Proposed patch with proper coding style
Can't we get a test case for this?
r- due to missing test case and because you should do the style clean up in a separate patch.
Created attachment 59434 [details]
Rediff against trunk
So did you remove the style update as I requested? and put it into another patch? (In reply to comment #7) > So did you remove the style update as I requested? and put it into another patch? I had to redo the patches since I had done them against the qtwebkit2.0 branch accidentally. In so doing many of the style issues had since been fixed on trunk - in this case the style issues I had fixed were gone. In another couple of patches I remove all extraneous style fixes and I will submit a separate bug against those. Comment on attachment 59434 [details] Rediff against trunk Rejecting patch 59434 from commit-queue. Failed to run "[u'git', u'svn', u'dcommit']" exit_code: 1 Last 500 characters of output: runk ... M WebCore/ChangeLog M WebCore/platform/graphics/qt/GraphicsLayerQt.cpp M WebCore/rendering/RenderLayerBacking.cpp A repository hook failed: MERGE request failed on '/repository/webkit/trunk': Commit blocked by pre-commit hook (exit code 1) with output: svnlook: Can't write to stream: Broken pipe The following ChangeLog files contain OOPS: trunk/WebCore/ChangeLog Please don't ever say "OOPS" in a ChangeLog file. at /usr/local/git/libexec/git-core/git-svn line 570 Full output: http://webkit-commit-queue.appspot.com/results/3277789 Created attachment 59777 [details]
Patch
Comment on attachment 59777 [details]
Patch
Forwarding previous R+.
Comment on attachment 59777 [details] Patch Rejecting patch 59777 from commit-queue. Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Adam Barth', u'--force']" exit_code: 1 Parsed 3 diffs from patch file(s). patching file WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file WebCore/platform/graphics/qt/GraphicsLayerQt.cpp Hunk #1 FAILED at 345. Hunk #2 FAILED at 356. Hunk #3 FAILED at 364. Hunk #4 succeeded at 566 (offset 80 lines). Hunk #5 succeeded at 578 (offset 81 lines). 3 out of 5 hunks FAILED -- saving rejects to file WebCore/platform/graphics/qt/GraphicsLayerQt.cpp.rej patching file WebCore/rendering/RenderLayerBacking.cpp Full output: http://webkit-commit-queue.appspot.com/results/3333773 Created attachment 59932 [details]
Patch
Comment on attachment 59932 [details]
Patch
WebCore/ChangeLog:7
+ that the pixemap has no alpha.
pixmap :-)
Looks fine to me. No'am?
Created attachment 59933 [details]
Patch
Comment on attachment 59933 [details] Patch > + if (rendererHasBackground()) { > + const Color &bgColor = rendererBackgroundColor(); > + m_graphicsLayer->setBackgroundColor(bgColor); > + } else > + m_graphicsLayer->clearBackgroundColor(); You can't do this. GraphicsLayers are not always the same size as their elements; in many cases, they are larger (e.g. try one with a box-shadow, or a abs. pos. child). I think you're really trying to fix bug 40629. *** This bug has been marked as a duplicate of bug 40629 *** |