33593 - fonts not antialiased with -moz-opacity

Status: VERIFIED FIXED

(Core Graveyard :: GFX, defect, P5)

Description

[Chris Hill]

Make sure font smoothing is turned on in Windows when trying to reproduce using 
attachment.

The new red/blue (I'll use read and blue for color1 and color2) buffer blender 
is much improved over the old one.  No more red fill in transparent areas.  
However, there is still a minor problem with the blending in the case of text 
with antialiased fonts.  The blender draws twice, once over a red background and 
once over a blue background.  It determines if a pixel is transparent by 
checking if the painting covered the red or the blue.  If the pixel is not red 
in the red buffer or it is not blue in the blue buffer it assumes that the pixel 
is to be included in the blend.  The difficulty with antialiased fonts is that 
they blend with the red and the blue to result in a change, but when the 
antialiased pixel is blended the red becomes part of the final pixel value.  The 
result is a red halo around the text.

The blender assumes that pixels that were modified are opaque.  Antialiased 
fonts do not fall under this assumption, some of the pixels they draw are not 
opaque.

Two possible solutions:
1) When opacity is < 1, turn off font antialiasing.  It doesn't look like the 
mozilla gfx stuff currently supports this.  Windows does support using a 
NONANTIALIASED_QUALITY in the LOGFONT structure, but this would need to be added 
to gfx.  I don't know about other platforms.  This would mean that text would 
look slightly different when opacity < 1, but that is probably better than the 
red halo.

2) When the blender is determining if a pixel is transparent or should be 
blended, it could compare the pixel values of the red and blue buffers after 
determining that at least one of the buffers has changed.  If they are 
different, assume that the difference is the result of blending and determine 
the original color and alpha of the pixel before it was blended with the red.  
With this information, blend the original color instead of the red tinted one 
with the destination.  This might be too much work to do in the inner loop of 
the blending operation, but it would produce in the best looking results.

Comment 1

[Chris Hill]

Attachment: Example demonstrating halo problem

Comment 2

[Kevin McCluskey (gone)]

reassigning to dcone.

Comment 3

[Henri Sivonen (:hsivonen)]

Possible solution #3:
Draw the text in a shade of gray corresponding to the opacity over a black background in 
an off-screen buffer. Use the result as and alpha channel with which color can be applied 
to the on-screen rendering.

(Consider a PNG with all pixels filled with the text color and the above-mentioned 
off-screen buffer as the alpha channel.)

Comment 4

[Chris Hill]

The code that calls the blender is not aware of the specific details of the 
contents of each view it renders.  If this trick was used for text, it would 
need to be duplicated for other drawing tasks that use transparent pixels (like 
drawing a PNG with an 8-bit alpha channel, which exhibits the same problem 
with opacity as antialiased fonts).  It would be nice if all the platforms 
supported destination alpha buffers so whenever drawing occured alpha would be 
stored in the graphics context along with the color components.  This support 
doesn't exist on most of the the target platforms, so the red/blue is probably 
the best than can be done without specific knowledge of each drawing process.

Comment 5

[Henri Sivonen (:hsivonen)]

Alpha channel already works with PNG files. If you want to make an image with alpha 
channel more transparent, you can basically just multiply the values of the alpha channel 
and then use the basic alpha blending code. (Well, gamma correction would add a 
couple of other operations.)

Comment 6

[Gervase Markham [:gerv]]

Confirming. Marking 4xp.

Gerv

Comment 7

[dcone (gone)]

This bug has been marked future because we have determined that it is not 
critical for netscape 6.0.  If you feel this is an error, or if it blocks your 
work in some way -- please attach your concern to the bug for reconsideration.

Comment 8

[Henri Sivonen (:hsivonen)]

*** Bug 42027 has been marked as a duplicate of this bug. ***

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Shouldn't be hard to modify the blender to make this work.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

*** Bug 46607 has been marked as a duplicate of this bug. ***

Comment 11

[Hixie (not reading bugmail)]

FWIW: This bug makes it basically impossible to use our 'opacity' extension with
inline elements. This is a minor issue though, since this is not a standards-
defined feature.

CSS extension. Taking QA.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

No, you can use "opacity" as long as the opaque thing is does not contain any 
transparent content. For example, it's fine to have a translucent 
element containing anti-aliased text (or an alpha PNG) as long as the 
translucent element also has a background color.

Having said that, this bug should not be hard to fix, although it'd probably be 
an interesting challenge to have the necessary blender loop run at high speed.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I have a fix to the blender which lets it handle views which used alpha-channel
rendering. It's not hard. It's even quite fast: two integer multiplies per pixel
per colour channel, in the worst case, which is the same as the current blender
code.

However, the current blender code could be improved to use only one integer
multiply per pixel per colour channel, for pixels that were drawn opaquely by
the view. My patch actually does this.

I will attach a patch once I've verified it against some more examples.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Patch to nsBlender and nsViewManager2 to allow blending of alpha-transparent views

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This should do the trick!

Comment 16

[Hixie (not reading bugmail)]

Robert tells me the patch will need to be updated when his nsViewManager2 patch
lands. This is therefore dependent on that other patch. Marked as such.

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

My nsViewManager2 changes are not going to land as such in the forseeable 
future, so I'm removing the dependency. This patch can be checked in now. Who 
can give me review and approval?

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can I get some cycles here? This bug is dead if I can just get review/approval.

Who is the mozilla.org contact for this component?

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

dcone, kmcclusk - See previous comment.

Comment 20

[Andrew Thompson]

Can we get this checked in please? its really visually horrible.

BTW Robert:

>No, you can use "opacity" as long as the opaque thing is does not contain any 
>transparent content. For example, it's fine to have a translucent 
>element containing anti-aliased text (or an alpha PNG) as long as the 
>translucent element also has a background colour.

Very probably I just don't understand you, but I can see the red halo on an 
element that has a background colour. In fact, given the opacity effects both the 
text and the background of an image, I find it hard to see how a background 
colour can prevent the element from "contain(ing) any trasparent content", given 
the colour itself will be transparent if the text is.

Unless there's a way to specify the foreground colour be transparent whilst the 
background colour is opaque? i.e. something like color-opacity: 50%; background-
opacity:100%;

Are the Mozilla css extensions documented anywhere?

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'm thinking of a document like this:

<html><body style="background-color: black">
    <div style="opacity: 50%; background-color: red; color: green;">
        Some text.
    </div>
</body></html>

This should render OK even with the current blender. The point is that the 
element would render all of its pixels opaquely if not for the opacity property, 
so blending works. Problems occur when the element renders some of its pixels 
partially opaque and then we blend to apply the opacity property, and things go 
haywire.

Because of the way views work, if the DIV contains something complex (e.g. a 
positioned element, or maybe an image) then things are probably going to lose 
again.

So yeah, it would be nice to have this checked in :-).

Comment 22

[Andrew Thompson]

>Because of the way views work, if the DIV contains something complex (e.g. a 
>positioned element, or maybe an image) then things are probably going to lose 
>again.

That would be it. I had fixed positioning in the mix...

Comment 23

[Hixie (not reading bugmail)]

Rubber stamping [nsbeta3-], since Robert's fix is slated for the next release
do to high risk considerations.

Comment 24

[Hixie (not reading bugmail)]

Somehow the red halo is gone, but anti-aliasing is gone too -- see:
   http://www.hixie.ch/tests/adhoc/css/mozilla/001.html
Robert: What's the status on this bug? I believe I'm running with your view
manager changes.

Comment 25

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This should be fixed with the blender changes in bug 42011. I don't have a
recent Windows build so I don't know what the status is there.

Comment 26

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The new blender code has been checked in. Someone running Windows needs to try 
to reproduce this bug with tomorrow's build.

Comment 27

[Hixie (not reading bugmail)]

Wooooo!! Anti-aliased semi-transparent text! Kick ass! VERIFIED.
Except that it hasn't been marked FIXED. Robert?

Comment 28

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I hadn't marked it fixed because I wasn't sure if it was fixed or not. Marking 
FIXED...

Comment 29

[Hixie (not reading bugmail)]

VERIFIED!!! Weeee!!!!