Summary: | [chromium] Enable LCD AA if text is painted inside opaque region | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alok Priyadarshi <alokp> | ||||
Component: | Layout and Rendering | Assignee: | Alok Priyadarshi <alokp> | ||||
Status: | RESOLVED WONTFIX | ||||||
Severity: | Normal | CC: | cc-bugs, danakj, d-r, enne, jamesr, junov, senorblanco, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | 104956 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Alok Priyadarshi
2012-12-14 12:37:43 PST
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 |