Closed
Bug 1055622
Opened 10 years ago
Closed 10 years ago
Add support for specifying the font smoothing background color to Moz2D
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: mstange, Assigned: mstange)
References
Details
Attachments
(2 files, 3 obsolete files)
CoreGraphics has a private API since 10.9 called CGContextSetFontSmoothingBackgroundColor which lets you have subpixel anti-aliased fonts on transparent backgrounds if you roughly know what the final background color is going to be. We'll need to call this if we want to have nice text rendering on top of "vibrant" widgets on 10.10, like the browser sidebar and context menus. And the cleanest way to support this is probably to start by adding support for it to Moz2D. Proposed API: class DrawTarget { ... bool SupportsFontSmoothingBackgroundColor() const; void SetFontSmoothingBackgroundColor(const Color& aColor); ... }
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
r?Bas on the 2d.h changes I'm not really thrilled to add another bit of state to DrawTarget, but putting the color into GlyphRenderingOptions looks quite tricky to me, especially from the Gecko side.
Assignee: nobody → mstange
Attachment #8499989 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8503136 -
Flags: review?(bas)
Assignee | ||
Comment 3•10 years ago
|
||
Comment on attachment 8503136 [details] [diff] [review] v1 r?jrmuizel on the DrawTargetCG changes, especially on the EnsureValidPremultipliedData ugliness
Attachment #8503136 -
Flags: review?(jmuizelaar)
Assignee | ||
Comment 4•10 years ago
|
||
This sheds some light on what CG is doing.
Comment 5•10 years ago
|
||
Comment on attachment 8503136 [details] [diff] [review] v1 Review of attachment 8503136 [details] [diff] [review]: ----------------------------------------------------------------- This should probably be added to the recording backends as well. ::: gfx/2d/DrawTargetCG.cpp @@ +1505,5 @@ > + // context into a context that has a different color space throws up on > + // invalid premultiplied data and creates completely wrong colors. > + // Sanitizing the data means that we lose some of the fake component alpha > + // behavior that font rendering tries to give us, but the result still > + // looks good enough to prefer it over grayscale font anti-aliasing. When do we draw into a context with a different color space? We should probably not ever be doing that. @@ +1698,5 @@ > + if (!lookedUpFunc) { > + void* coreGraphicsFramework = dlopen(COREGRAPHICS_FRAMEWORK_PATH, RTLD_LAZY | RTLD_LOCAL); > + if (coreGraphicsFramework) { > + func = (CGContextSetFontSmoothingBackgroundColorFunc)dlsym( > + coreGraphicsFramework, "CGContextSetFontSmoothingBackgroundColor"); I think you can just use RTLD_DEFAULT instead of coreGraphicsFramework here. That's what ScaledFontMac::ScaledFontMac does. @@ +1712,5 @@ > +{ > + DrawTarget::SetFontSmoothingBackgroundColor(aColor); > + > + CGContextSetFontSmoothingBackgroundColorFunc func = > + GetCGContextSetFontSmoothingBackgroundColorFunc(); Use a better name than func. Do you have some idea what this function is actually doing? If you do a description in a comment someplace would be great.
Attachment #8503136 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Jeff Muizelaar [:jrmuizel] from comment #5) > Comment on attachment 8503136 [details] [diff] [review] > v1 > > Review of attachment 8503136 [details] [diff] [review]: > ----------------------------------------------------------------- > > This should probably be added to the recording backends as well. right, thanks > ::: gfx/2d/DrawTargetCG.cpp > @@ +1505,5 @@ > > + // context into a context that has a different color space throws up on > > + // invalid premultiplied data and creates completely wrong colors. > > + // Sanitizing the data means that we lose some of the fake component alpha > > + // behavior that font rendering tries to give us, but the result still > > + // looks good enough to prefer it over grayscale font anti-aliasing. > > When do we draw into a context with a different color space? We should > probably not ever be doing that. When drawing into the context we get from drawRect, at least. I saw the bad colors in context menus, which don't use hardware acceleration. You can also see it happening in the test app I attached. > @@ +1698,5 @@ > > + if (!lookedUpFunc) { > > + void* coreGraphicsFramework = dlopen(COREGRAPHICS_FRAMEWORK_PATH, RTLD_LAZY | RTLD_LOCAL); > > + if (coreGraphicsFramework) { > > + func = (CGContextSetFontSmoothingBackgroundColorFunc)dlsym( > > + coreGraphicsFramework, "CGContextSetFontSmoothingBackgroundColor"); > > I think you can just use RTLD_DEFAULT instead of coreGraphicsFramework here. > That's what ScaledFontMac::ScaledFontMac does. Oh good. "man dlsym" says about RTLD_DEFAULT "This can be a costly search and should be avoided.", but if we're already doing it then I'll just ignore that warning. > > @@ +1712,5 @@ > > +{ > > + DrawTarget::SetFontSmoothingBackgroundColor(aColor); > > + > > + CGContextSetFontSmoothingBackgroundColorFunc func = > > + GetCGContextSetFontSmoothingBackgroundColorFunc(); > > Use a better name than func. ok > Do you have some idea what this function is actually doing? As in, do I know the actual calculation? No, I don't know that. I only know what I wrote in the first paragraph of comment 0. > If you do a > description in a comment someplace would be great. ok
Assignee | ||
Comment 7•10 years ago
|
||
Attachment #8503244 -
Flags: review?(bas)
Comment 8•10 years ago
|
||
Comment on attachment 8503136 [details] [diff] [review] v1 Review of attachment 8503136 [details] [diff] [review]: ----------------------------------------------------------------- This is so very specific to a certain backend, I feel it should go into a GlyphRenderingOptions object rather than through its own API. These sort of things will really clutter the API.
Attachment #8503136 -
Flags: review?(bas) → review-
Assignee | ||
Updated•10 years ago
|
Attachment #8503244 -
Flags: review?(bas)
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8503136 -
Attachment is obsolete: true
Attachment #8503244 -
Attachment is obsolete: true
Attachment #8505725 -
Flags: review?(bas)
Updated•10 years ago
|
Attachment #8505725 -
Flags: review?(jmuizelaar)
Attachment #8505725 -
Flags: review?(bas)
Attachment #8505725 -
Flags: review+
Updated•10 years ago
|
Attachment #8505725 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 10•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d593636d86d
https://hg.mozilla.org/mozilla-central/rev/1d593636d86d
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•