rgba text is broken on win32

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Core
Graphics
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: philippe (part-time), Assigned: jrmuizel)

Tracking

({regression, testcase})

Trunk
mozilla1.9.2a1
x86
Windows 7
regression, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

9 years ago
Created attachment 369057 [details]
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.
(Reporter)

Comment 1

9 years ago
Created attachment 369058 [details]
test case
(Reporter)

Comment 2

9 years ago
Created attachment 369060 [details]
correct test case
Attachment #369058 - Attachment is obsolete: true

Comment 3

9 years ago
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
slightly better range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e1120120b1b7&tochange=d14de45019a0
Keywords: testcase
Flags: blocking1.9.2?
(Reporter)

Comment 5

9 years ago
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
(Assignee)

Comment 7

9 years ago
I can reproduce this on vista, which makes it more likely to have been caused by the cairo update.
(Assignee)

Comment 8

9 years ago
It looks like any text with an rgba color is broken on win32.
(Assignee)

Comment 9

9 years ago
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)

Updated

9 years ago
Assignee: nobody → jmuizelaar
(Assignee)

Comment 10

9 years ago
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.
(Assignee)

Comment 11

9 years ago
Created attachment 376074 [details] [diff] [review]
Propogate the component alpha flag when wrapping image surfaces

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?
(Assignee)

Updated

9 years ago
Attachment #376074 - Flags: review? → review?(vladimir)
Attachment #376074 - Flags: review?(vladimir) → review+
Comment on attachment 376074 [details] [diff] [review]
Propogate the component alpha flag when wrapping image surfaces

Looks good to me
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
(Assignee)

Comment 13

9 years ago
Created attachment 376280 [details] [diff] [review]
v2 with the correct patches against upstream
[Checkin: Comment 15]
Attachment #376074 - Attachment is obsolete: true
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
Last Resolved: 9 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
(Reporter)

Comment 16

9 years ago
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.
(Reporter)

Comment 18

9 years ago
(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).
(Assignee)

Comment 19

9 years ago
Yeah, I screwed up here. I'll see what's going on...
(Assignee)

Comment 20

9 years ago
Created attachment 377166 [details] [diff] [review]
Fix a typo

The patch that was committed has a typo. It was getting the component_alpha flag from destination surface instead of the source surface.
(Assignee)

Updated

9 years ago
Attachment #377166 - Flags: review?(vladimir)
Attachment #377166 - Flags: review?(vladimir) → review+
Can we add a reftest or a mochitest for this somehow?
(Assignee)

Updated

9 years ago
Whiteboard: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/dd19a3546676

The reftest is bug 493067.
Whiteboard: checkin-needed
(Reporter)

Comment 23

9 years ago
(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
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.