We already mark layers whether they can use LCD text based on their transform and opacity. Now actually paint LCD text if it lies inside opaque region.
Created attachment 179517 [details] Patch
Uploading for early feedback. I still need to add unit-test for OpaqueRegionSkia::isOpaqueInRect(). This version of patch always uses NULL rect for text. So it only enables LCD on layers that have an opaque background (including root layer).
Comment on attachment 179517 [details] Patch OpaqueRegionSkia changes look good. It would be nice to use/verify isOpaqueInRect() in the PlatformContextSkia tests.
Comment on attachment 179517 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=179517&action=review Won't this produce inconsistently rendered text within a layer depending on how the paint rect interacts with the opaque region? This patch doesn't appear to actually populate the rect at all, so I can't tell where it is going to come from in general. > Source/WebCore/platform/graphics/skia/PlatformContextSkia.cpp:449 > +bool PlatformContextSkia::adjustPaintFlagsForText(SkPaint* paint, const SkRect* rect) const I can't find any callers that pass in a non-null rect parameter - where are they?
(In reply to comment #2) > Uploading for early feedback. I still need to add unit-test for OpaqueRegionSkia::isOpaqueInRect(). > > This version of patch always uses NULL rect for text. So it only enables LCD on layers that have an opaque background (including root layer). Ah, I see. So this doesn't actually do rect-region comparisons. When it does, where do you anticipate the rect being compared to the opaque region will come from?
Comment on attachment 179517 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=179517&action=review >> Source/WebCore/platform/graphics/skia/PlatformContextSkia.cpp:449 >> +bool PlatformContextSkia::adjustPaintFlagsForText(SkPaint* paint, const SkRect* rect) const > > I can't find any callers that pass in a non-null rect parameter - where are they? They will come from estimating text bounds using something like this: http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/chromium/FontChromiumWin.cpp#L240 I chose to not compute bounds in this patch because text drawing code scattered in platform-specific files. I will add them for each platform in separate patches. But if you prefer I can get rid of the rect parameter for now. I could just always check device-clip-rect.
(In reply to comment #4) > (From update of attachment 179517 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=179517&action=review > > Won't this produce inconsistently rendered text within a layer depending on how the paint rect interacts with the opaque region? > Yes this is a possibility. There is no way to un-paint LCD text in immediate mode. We may be able to avoid it with impl-side painting where we can defer the decision until after recording the SkPicture.
I have decided to abandon this patch and move the refactoring changes to two separate patches here: https://bugs.webkit.org/show_bug.cgi?id=106266 https://bugs.webkit.org/show_bug.cgi?id=106267