WebGL: Y axis is upside down

RESOLVED FIXED

Status

Firefox for Android Graveyard
General
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: paul, Assigned: vlad)

Tracking

Trunk
ARM
Android

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

8 years ago
WebGL on Fennec/Android is upside down.
Example:
http://learningwebgl.com/lessons/lesson14/index.html

The object is upside down on Fennec.

Updated

8 years ago
tracking-fennec: --- → ?
Assignee: nobody → bjacob
tracking-fennec: ? → 2.0b3+

Comment 1

8 years ago
Problem can be reproduced on Fennec/Mac.
Vladimir, what is the GL provider on Fennec? EGL? So can I assume that all what's needed is to flip the display in EGL?
Created attachment 490685 [details] [diff] [review]
Set y-flip always for remote canvas
Attachment #490685 - Flags: review?(bjacob)
Attachment #490685 - Flags: review?(bjacob) → review+
Comment on attachment 490685 [details] [diff] [review]
Set y-flip always for remote canvas

Comment from cjones
(In reply to comment #3)
> Comment on attachment 490656 [details] [diff] [review]
> Add y-flip for remote sw canvas
> 
> Don't we only want to y-flip for webgl canvases?  Remember that this code is
> used for both 2d and 3d.
Attachment #490685 - Flags: review-
Created attachment 490725 [details] [diff] [review]
Should be better y-flip handling
Assignee: bjacob → romaxa
Attachment #490685 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #490725 - Flags: review?(jones.chris.g)
WebGL still isn't blocking fennec 2.0.  I don't want to take this patch until we get things stable on all sides (desktop WebGL, Fennec GL layers, Fennec remote layers, etc.).
tracking-fennec: 2.0b3+ → ---
Blocks: 619056
Duplicate of this bug: 619328
Comment on attachment 490725 [details] [diff] [review]
Should be better y-flip handling

Not the right patch; the data should already be flipped correctly before passed over the wire.
Attachment #490725 - Flags: review?(jones.chris.g) → review-
Created attachment 504075 [details] [diff] [review]
always pass right-side-up surface across the wire

The code was blindly using mSurface before, when rendering needs to take into account flip here.  We can just call Paint(), which will do the right thing.

We also skip calling Paint() needlessly when we're shadowed.
Attachment #504075 - Flags: review?(jones.chris.g)
Comment on attachment 504075 [details] [diff] [review]
always pass right-side-up surface across the wire

The DrawSurface->Paint fix looks OK, but we can't skip Paint(aContext...) when HasShadow() in general because the target might be for a drawWindow().  I wouldn't bother with that unless you have evidence that painting to the dummy context is going to incur the kind of overhead that would show up in profiles.  Note we do the same thing for thebeslayers, and that would be more important to optimize.  I'd like to fix that silliness in general but evidence hasn't pointed to it being an issue yet (I thought I'd filed a bug too but I can't find it).

Nit: the "call ... again" comment is somewhat misleading, in that the "again" is a non sequitur.
Attachment #504075 - Flags: review?(jones.chris.g) → review-
Created attachment 504080 [details] [diff] [review]
v2

ugh, didn't realize that drawwindow also used it.  ok, just the main fix.
Attachment #490725 - Attachment is obsolete: true
Attachment #504075 - Attachment is obsolete: true
Attachment #504080 - Flags: review?(jones.chris.g)
Comment on attachment 504080 [details] [diff] [review]
v2

Would still s/again// in the comment, but suits me.
Attachment #504080 - Flags: review?(jones.chris.g) → review+
http://hg.mozilla.org/mozilla-central/rev/809aded51aad

.. but when I looked the code over, I think this might end up double-applying opacity, because opacity is a layer property that gets forwarded, right?  So we might have to set opacity to 1.0 before calling the second Paint.
Hm yeah, my bad for not catching.  We'll DrawSurface() using OPERATOR_SOURCE with the layer opacity, which if I read cairo docs correctly says it will apply the opacity.  Could probably refactor Paint() to another method that takes an opacity param, without too much pain.
Yeah, I'll post a followup here shortly.
Created attachment 506187 [details] [diff] [review]
fix opacity issue with earlier patch

Here's a fix for the opacity issue.  I did the refactoring as suggested.
Assignee: romaxa → vladimir
Attachment #506187 - Flags: review?(jones.chris.g)
Comment on attachment 506187 [details] [diff] [review]
fix opacity issue with earlier patch

># HG changeset patch
># Parent ff8f838c15e835db62595ce216fe6bc4bdb4bbe5
>diff --git a/gfx/layers/basic/BasicLayers.cpp b/gfx/layers/basic/BasicLayers.cpp
>--- a/gfx/layers/basic/BasicLayers.cpp
>+++ b/gfx/layers/basic/BasicLayers.cpp
>@@ -820,6 +820,11 @@ public:
>                      LayerManager::DrawThebesLayerCallback aCallback,
>                      void* aCallbackData);
> 
>+  virtual void PaintWithOpacity(gfxContext* aContext,
>+                                LayerManager::DrawThebesLayerCallback aCallback,
>+                                void* aCallbackData,

Don't need the thebes-callback params.

r=me with that, otherwise looks good.  Yay, comments.
Attachment #506187 - Flags: review?(jones.chris.g) → review+
http://hg.mozilla.org/mozilla-central/rev/44ab59b32b35
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Duplicate of this bug: 628604

Comment 20

7 years ago
The bug is fixed on trunk:

Build Id: Mozilla /5.0 (Android;Linux armv7l;rv:7.0a1) Gecko/20110608 Firefox/7.0a1 Fennec/7.0a1 

But on Aurora build, I cannot see the object spinning. I just see the black square. Will it be also implemented in Aurora?

Should I mark it as VERIFIED FIXED, if on Aurora is not working?
Do we have decode_on_draw fix applied on Aurora? see bug https://bugzilla.mozilla.org/show_bug.cgi?id=647802
set "image.mem.decodeondraw" -> false on android and check, if that works, then we should land also 647802
You need to log in before you can comment on or make changes to this bug.