Closed
Bug 975121
Opened 11 years ago
Closed 11 years ago
Linux - reversed colors with gfx.xrender.enabled = false
Categories
(Core :: Graphics: Layers, defect)
Core
Graphics: Layers
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: johns, Assigned: mtseng)
References
Details
Attachments
(1 file, 1 obsolete file)
1.19 KB,
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
With gfx.xrender.enabled set to false* and hardware acceleration enabled, I've noticed sporadicly reversed colors in linux. I bisected the culprit to:
http://hg.mozilla.org/mozilla-central/rev/c0e256be477547426276d2a9d078e7a4c6c1729b
This occured once before with linux OMTC: Bug 895954
Which :mattwoodrow tracked down in bug 910488 comment 5
Reporter | ||
Comment 1•11 years ago
|
||
A site I've found to reliably reproduce is:
https://github.com/mozilla/
The project links show up orange, instead of blue.
Assignee | ||
Comment 2•11 years ago
|
||
Need to check GetPreferredARGB32Format in this condition for correct result.
try push: https://tbpl.mozilla.org/?tree=Try&rev=72a5076ed23e
Assignee: nobody → mtseng
Attachment #8379473 -
Flags: review?(gal)
Comment 3•11 years ago
|
||
Comment on attachment 8379473 [details] [diff] [review]
check_GetPreferredARGB32Format
Review of attachment 8379473 [details] [diff] [review]:
-----------------------------------------------------------------
I don't think we can rely on this being correct.
TextureSource objects have a format (GetFormat()) that canonically defines what format (and thus byte ordering) the data is in.
What should use this information (possibly by storing it in the Effect) to determine what swizzling the shader needs to do.
Assignee | ||
Comment 4•11 years ago
|
||
mattwoodrow, you're right. We have information from TextureSource. re-upload patch to use this info. Thanks!
try push: https://tbpl.mozilla.org/?tree=Try&rev=96863754f11a
Attachment #8379565 -
Flags: review?(gal)
Assignee | ||
Updated•11 years ago
|
Attachment #8379473 -
Attachment is obsolete: true
Attachment #8379473 -
Flags: review?(gal)
Comment 5•11 years ago
|
||
Comment on attachment 8379565 [details] [diff] [review]
check_rb_swap_for_component_alpha
Fun bug :)
Attachment #8379565 -
Flags: review?(gal) → review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment on attachment 8379565 [details] [diff] [review]
check_rb_swap_for_component_alpha
Review of attachment 8379565 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/layers/opengl/CompositorOGL.cpp
@@ +828,4 @@
> config.SetComponentAlpha(true);
> + EffectComponentAlpha* effectComponentAlpha =
> + static_cast<EffectComponentAlpha*>(aEffect);
> + gfx::SurfaceFormat format = effectComponentAlpha->mOnWhite->GetFormat();
I am not quite familiar with component_alpha effect.
Logic here is use the format of mOnWhite to decide whether apply swizzling.
it swaps both RB channel of mOnWhite and mOnBlack in pixel shader. We assume that the format of mOnWhite and mOnBlack is the same. If this assumption is wrong(for example, mOnWhite is B8G8R8A8 and mOnBlack is R8G8B8A8), thing goes wrong.
I suggest add a assertion here.
MOZ_ASSERT(effectComponentAlpha->mOnWhite->GetFormat() == effectComponentAlpha->mOnBlack->GetFormat());
Comment 7•11 years ago
|
||
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in
before you can comment on or make changes to this bug.
Description
•