Closed Bug 484954 Opened 11 years ago Closed 11 years ago

rgba text is broken on win32

Categories

(Core :: Graphics, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: phiw2, Assigned: jrmuizel)

References

Details

(Keywords: regression, testcase)

Attachments

(4 files, 2 obsolete files)

Attached image screenshot of testcase
text-shadow: 1px 2px 0 rgba(250,250,250, 0.65);

replace rgba() with rgb() and it works fine.
if the blur value is larger than 1px, it displays fine.

This seems limited to Windows 7.
I'm not sure about the regression range. However, I'm sure I was doing tests on this with the 20090319 build, where it worked fine.
Possibly the Cairo/Pixman upgrade ? (bug  484076)

Displays correctly on OS X and Linux, btw.
Attached file test case (obsolete) —
Attached file correct test case
Attachment #369058 - Attachment is obsolete: true
Same issue on Windows XP.
Regression range:
Works:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090320 Minefield/3.6a1pre

Broken
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090321 Minefield/3.6a1pre

Changesets:
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2009-03-20+04%3A50%3A46&enddate=2009-03-21+05%3A05%3A53
Flags: blocking1.9.2?
Thanks for checking the regression range. bug 484076 seems the most likely candidate.
Blocks: 484076
Jeff, this might be a regression from the Cairo upgrade. Can you check?
Component: Layout → GFX: Thebes
QA Contact: layout → thebes
I can reproduce this on vista, which makes it more likely to have been caused by the cairo update.
It looks like any text with an rgba color is broken on win32.
This seems to be caused by the source image wrapping.
Summary: Text-shadow using rgba() for color value displays with ugly spread → rgba text is broken on win32
Assignee: nobody → jmuizelaar
It is. 

It turns out we don't propagate the component_alpha flag and so the surface gets treated as regular argb32 surface and the alpha values are used instead of the component alpha values.
Not setting the component alpha flag on the pixman image was causing us to misinterpret the pixman image mask.

The patch doesn't do much more then pixman_image_set_composite_alpha(pixman_image_get_composite_alpha())

The rest of the diff is due to keeping around a cairo_image_surface_t instead of a cairo_surface_t to avoid a lot of casting.

I'll update the associated wrap-source.patch before this gets committed.
Attachment #376074 - Flags: review?
Attachment #376074 - Flags: review? → review?(vladimir)
Comment on attachment 376074 [details] [diff] [review]
Propogate the component alpha flag when wrapping image surfaces

Looks good to me
Keywords: checkin-needed
Comment on attachment 376280 [details] [diff] [review]
v2 with the correct patches against upstream
[Checkin: Comment 15]

This just adds the changes to our explicit patches against cairo.
Attachment #376280 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/89338a224278

I committed the patch on Thursday, but forgot to mark this bug as fixed.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Attachment #376280 - Attachment description: v2 with the correct patches against upstream → v2 with the correct patches against upstream [Checkin: Comment 15]
Flags: blocking1.9.2?
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9.2a1
This is not yet fixed, or regressed again.
The attached test case still shows the ugly spread when using rgba() text shadow or rgba() for the color property on Windows 7 and Vista with

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090512 Minefield/3.6a1pre (.NET CLR 3.5.30729)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Did the patch have an observable effect on the testcase?

Unless the patch needs backing out (Jeff?), you should prefer filing a new bug over REOPENing a FIXED one.
(In reply to comment #17)
> Did the patch have an observable effect on the testcase?

I don't think so.
I fetched the first official nightly after that patch landed: 
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090508 Minefield/3.6a1pre (.NET CLR 3.5.30729)
And the test case, along with some others I have on my dev server, fails.

> 
> Unless the patch needs backing out (Jeff?), you should prefer filing a new bug
> over REOPENing a FIXED one.

Well, apparently, the patch as landed didn't fix the original issue (rba() text-shadow) as filed. Nor did it fix the problem with rgba() color mentioned in comment 8.

----
per comments in the forums, the issue can be seen on all Windows versions (XP, Vista, Windows 7 RC).
Yeah, I screwed up here. I'll see what's going on...
Attached patch Fix a typoSplinter Review
The patch that was committed has a typo. It was getting the component_alpha flag from destination surface instead of the source surface.
Attachment #377166 - Flags: review?(vladimir)
Can we add a reftest or a mochitest for this somehow?
Whiteboard: checkin-needed
Whiteboard: checkin-needed
(In reply to comment #22)
> http://hg.mozilla.org/mozilla-central/rev/dd19a3546676
> 
> The reftest is bug 493067.

Works nicely now. Cool.

Shall I mark this as FIXED ? Or is there something more to do here ?

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090515 Minefield/3.6a1pre (.NET CLR 3.5.30729)
Fixed it is!
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.