Linux - reversed colors with gfx.xrender.enabled = false

RESOLVED FIXED in mozilla30

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: johns, Assigned: mtseng)

Tracking

unspecified
mozilla30
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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
A site I've found to reliably reproduce is:
https://github.com/mozilla/

The project links show up orange, instead of blue.
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 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.
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)
Attachment #8379473 - Attachment is obsolete: true
Attachment #8379473 - Flags: review?(gal)
Comment on attachment 8379565 [details] [diff] [review]
check_rb_swap_for_component_alpha

Fun bug :)
Attachment #8379565 - Flags: review?(gal) → review+
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());
https://hg.mozilla.org/mozilla-central/rev/5fa7f5df67b3
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Duplicate of this bug: 976425
You need to log in before you can comment on or make changes to this bug.