New issue
Advanced search Search tips

Issue 877159 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocked on:
issue 872845



Sign in to add a comment

WebGL renders incorrectly with GL_MESA_framebuffer_flip_y

Project Member Reported by frkoenig@chromium.org, Aug 23

Issue description

Chrome Version: 70.0.3511 (Official Build) unknown (64-bit)
OS: Chrome

All of the work to get WebGL onto a plane for ChromeOS was done in drawing_buffer.cc.  There are some sites that are not displaying corectly with that work done.  It's not clear if it is in WebGL (webgl_rendering_context_base.cc), MESA, or somewhere else.

Sketchfab:
https://sketchfab.com/models/4918731850354b25a66a99ae149ca959

(pointed out here: https://bugs.chromium.org/p/chromium/issues/detail?id=680851&desc=2#c28 )

Mozilla bug:
https://bug818810.bmoattachments.org/attachment.cgi?id=689110
 
Owner: frkoenig@chromium.org
Status: Started (was: Untriaged)
Turns out there are conformance tests : https://www.khronos.org/registry/webgl/sdk/tests/webgl-conformance-tests.html

Currently failing some of them.
Blockedon: 872845
Is it likely that the Mesa bug in  Issue 872845  is related?

Should we blacklist this extension (see https://cs.chromium.org/chromium/src/gpu/config/gpu_driver_bug_list.json ) until a version of Mesa ships containing all the bug fixes?

I don't think 872845 is related.  That one traces back to an issue with the mesa driver.  Fixing that one didn't fix this one.

I'm hoping this can make M70, but I guess this is the time to figure out contingency plans.  Would a flag make more sense so that it can still be seen by outside users?  It seems like the blacklist would make it so no external users could use it at all.
Cc: zmo@chromium.org
If this isn't a Mesa bug but rather a bug in Chrome then let's work quickly to try to figure this one out. I don't think it's a good idea for long-term maintenance to expose a flag for the use of this one extension.

This has to do with the how gl_FragCoord operates.  It does not currently respect the framebuffer orientation in mesa and therefore does not respect GL_MESA_framebuffer_flip_y.

I have a solution and will be cleaning it up and landing it first in here, and then pushing it to upstream.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c1207bb85c5070974226524566ee2d6c200bbc10

commit c1207bb85c5070974226524566ee2d6c200bbc10
Author: Fritz Koenig <frkoenig@chromium.org>
Date: Tue Aug 28 23:06:52 2018

webgl: CopyToPlatformTexture needs to respect is_origin_top_left_

Fixes inversion when using toDataUrl with GL_MESA_framebuffer_flip_y.

BUG= 877159 
TEST=khronos webgl conformance tests in browser

Change-Id: Ic6d765db7d7f02b6c1ec0f9996994f3851e721a0
Reviewed-on: https://chromium-review.googlesource.com/1194787
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Fritz Koenig <frkoenig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586914}
[modify] https://crrev.com/c1207bb85c5070974226524566ee2d6c200bbc10/third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/7e82bab7afd5fbda29482234e833e299e8a25298

commit 7e82bab7afd5fbda29482234e833e299e8a25298
Author: Fritz Koenig <frkoenig@google.com>
Date: Wed Aug 29 23:09:21 2018

media-libs/mesa: GL_MESA_framebuffer_flip_y gl_FragCoord

gl_FragCoord needs to have the origin changed when
GL_MESA_framebuffer_flip_y is applied.

BUG= chromium:877159 
TEST=khronos webgl conformance tests in browser

Change-Id: Idc2657ac07c95b29b601540b676073cd69dad03a
Reviewed-on: https://chromium-review.googlesource.com/1194642
Commit-Ready: Fritz Koenig <frkoenig@chromium.org>
Tested-by: Fritz Koenig <frkoenig@chromium.org>
Reviewed-by: Kristian H. Kristensen <hoegsberg@chromium.org>

[modify] https://crrev.com/7e82bab7afd5fbda29482234e833e299e8a25298/media-libs/mesa/mesa-9999.ebuild
[modify] https://crrev.com/7e82bab7afd5fbda29482234e833e299e8a25298/media-libs/mesa/mesa-18.2_pre1.ebuild
[add] https://crrev.com/7e82bab7afd5fbda29482234e833e299e8a25298/media-libs/mesa/files/18.4-i965-Provide-a-mechanism-to-flip-fs-coordinate-syste.patch
[rename] https://crrev.com/7e82bab7afd5fbda29482234e833e299e8a25298/media-libs/mesa/mesa-18.2_pre1-r15.ebuild

Status: Fixed (was: Started)
fixed in cros R70-11017.0.0
Labels: Merge-Request-70
The original fix for gl_FragCoord was incorrect.  It is most likely the root cause of b/114654742.  The subsequent revert and correct fix should be moved to M70
Project Member

Comment 12 by sheriffbot@chromium.org, Oct 3

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: We are only 12 days from stable.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Fritz: just to save time with the project managers' approval, are you specifically requesting merge-back of the following two commits?

d617d30261d25757e5376b4813245283e75b57fa
aab1b24f79d4a60aa9739688398f2eb81e654e6b

Thanks.

Yes, those are the 2 commits
Labels: -Merge-Review-70 Merge-Approved-70
Project Member

Comment 16 by bugdroid1@chromium.org, Oct 4

Labels: merge-merged-release-R70-11021.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/f81c08e29b836f5dc5ee27473ab31bbab3abe4a8

commit f81c08e29b836f5dc5ee27473ab31bbab3abe4a8
Author: Fritz Koenig <frkoenig@google.com>
Date: Thu Oct 04 18:01:24 2018

Revert "media-libs/mesa: GL_MESA_framebuffer_flip_y gl_FragCoord"

There is a better/more correct way of achieving the
flip of glFragCoord.

BUG= chromium:877159 

This reverts commit 7e82bab7afd5fbda29482234e833e299e8a25298.

(cherry picked from commit d617d30261d25757e5376b4813245283e75b57fa)

Change-Id: Ie8f49df0d9d61bf47f187b03d591cc538608f9f8
Reviewed-on: https://chromium-review.googlesource.com/c/1262156
Trybot-Ready: Fritz Koenig <frkoenig@chromium.org>
Reviewed-by: Kristian H. Kristensen <hoegsberg@chromium.org>
Tested-by: Fritz Koenig <frkoenig@chromium.org>
Commit-Queue: Fritz Koenig <frkoenig@chromium.org>

[modify] https://crrev.com/f81c08e29b836f5dc5ee27473ab31bbab3abe4a8/media-libs/mesa/mesa-9999.ebuild
[modify] https://crrev.com/f81c08e29b836f5dc5ee27473ab31bbab3abe4a8/media-libs/mesa/mesa-18.2_pre1.ebuild
[delete] https://crrev.com/585331abd10c14fbd75abef287b6ebc8b489e0b0/media-libs/mesa/files/18.4-i965-Provide-a-mechanism-to-flip-fs-coordinate-syste.patch
[rename] https://crrev.com/f81c08e29b836f5dc5ee27473ab31bbab3abe4a8/media-libs/mesa/mesa-18.2_pre1-r17.ebuild

fixed in R70-11021.42.0 on soraka after the cherry-pick
Project Member

Comment 19 by sheriffbot@chromium.org, Oct 8

Cc: geo...@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Approved-70

Sign in to add a comment