Summary: | [Chromium] With the skia port, setting LCD text filtering is causing texture cache invalidations of gpu canvas backing store | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Justin Novosad <junov> | ||||||
Component: | Canvas | Assignee: | Justin Novosad <junov> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cc-bugs, jamesr, jin.a.yang, mdelaney7, reed, senorblanco, vangelis, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Windows 7 | ||||||||
Attachments: |
|
Description
Justin Novosad
2011-12-09 07:22:50 PST
The main use case where this causes trouble is with canvas to canvas draws from a software canvas to a GPU accelerated canvas. The backing store of the source canvas gets cached on the GPU. Bug 73186 is also related to the generation id management. Created attachment 135840 [details]
Patch
(In reply to comment #3) > Created an attachment (id=135840) [details] > Patch In this patch I removed all unnecessary uses of LayerIter that could be replaced with SkCanvas::isDrawingToLayer, not just the one in SkiaFontWin.cpp Comment on attachment 135840 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135840&action=review > Source/WebCore/platform/graphics/chromium/FontChromiumWin.cpp:143 > + } else if (m_createdTransparencyLayer || m_platformContext->canvas()->isDrawingToLayer()) { None of these changes are functionally the same, however. The old code checked if there was > 1 layer present. The new code checks if there is > 0 layers present. (In reply to comment #5) > (From update of attachment 135840 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=135840&action=review > > > Source/WebCore/platform/graphics/chromium/FontChromiumWin.cpp:143 > > + } else if (m_createdTransparencyLayer || m_platformContext->canvas()->isDrawingToLayer()) { > > None of these changes are functionally the same, however. The old code checked if there was > 1 layer present. The new code checks if there is > 0 layers present. Actually, behavior is the same. It is a little confusing, but the fLayerCount in SkCanvas represents the number of saved layers, which does not include the root layer. (In reply to comment #6) > Actually, behavior is the same. It is a little confusing, but the fLayerCount in SkCanvas represents the number of saved layers, which does not include the root layer. There is clearly a readability issue here. I think I will rename fLayerCount to fSavedLayerCount, to make things clearer in SkCanvas. Comment on attachment 135840 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135840&action=review >>> Source/WebCore/platform/graphics/chromium/FontChromiumWin.cpp:143 >>> + } else if (m_createdTransparencyLayer || m_platformContext->canvas()->isDrawingToLayer()) { >> >> None of these changes are functionally the same, however. The old code checked if there was > 1 layer present. The new code checks if there is > 0 layers present. > > Actually, behavior is the same. It is a little confusing, but the fLayerCount in SkCanvas represents the number of saved layers, which does not include the root layer. I see, thanks for the clarification. Making that more clear in Skia would be great :) Comment on attachment 135840 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135840&action=review > Source/WebKit/chromium/src/WebFontImpl.cpp:116 > + if (canvasIsOpaque && SkColorGetA(color) == 0xFF && !const_cast<SkCanvas*>(canvas)->isDrawingToLayer()) { Not your fault, but is this const_cast necessary anymore? isDrawingToLayer() is const. Created attachment 135858 [details]
Patch
Comment on attachment 135858 [details]
Patch
Looks good! r=me
Comment on attachment 135858 [details] Patch Clearing flags on attachment: 135858 Committed r113345: <http://trac.webkit.org/changeset/113345> All reviewed patches have been landed. Closing bug. |