Closed Bug 1249189 Opened 4 years ago Closed 4 years ago

Fix YCbCr convert problem when using TexImage2D

Categories

(Core :: Canvas: WebGL, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: ethlin, Assigned: ethlin)

References

()

Details

Attachments

(1 file, 1 obsolete file)

The related conformance test is:
video/tex-image-and-sub-image-2d-with-video-rgb8-rgb-unsigned_byte,
video/tex-image-and-sub-image-2d-with-video-srgb8-rgb-unsigned_byte, etc.

error messages:
testing: video/webm; codecs="vp8, vorbis"
Testing texImage2D with flipY=true bindingTarget=TEXTURE_2D
Checking lower left corner
FAIL at (4, 4) expected: 0,255,0 was 255,125,255
Checking upper left corner
FAIL at (4, 24) expected: 255,0,0 was 255,125,255
Testing texImage2D with flipY=false bindingTarget=TEXTURE_2D
Checking lower left corner
FAIL at (4, 4) expected: 255,0,0 was 255,125,255
Checking upper left corner
FAIL at (4, 24) expected: 0,255,0 was 255,125,255
Testing texSubImage2D with flipY=true bindingTarget=TEXTURE_2D
Checking lower left corner
FAIL at (4, 4) expected: 0,255,0 was 255,125,255
Checking upper left corner
FAIL at (4, 24) expected: 255,0,0 was 255,125,255
Testing texSubImage2D with flipY=false bindingTarget=TEXTURE_2D
Checking lower left corner
FAIL at (4, 4) expected: 255,0,0 was 255,125,255
Checking upper left corner
FAIL at (4, 24) expected: 0,255,0 was 255,125,255
...
Attached patch Fix YCbCr conversion. r=jgilbert (obsolete) — Splinter Review
The ALPHA, LUMINANCE, LUMINANCE_ALPHA in ES3.0.4 and OpenGL4.0 are legacy pixel formats. So I use GL_RED to do the conversion.
Attachment #8721884 - Flags: review?(jgilbert)
Comment on attachment 8721884 [details] [diff] [review]
Fix YCbCr conversion. r=jgilbert

Review of attachment 8721884 [details] [diff] [review]:
-----------------------------------------------------------------

RED textures aren't valid in GLES2, and ALPHA textures are not valid in desktop GL Core Profiles (relevant for OSX). However, R8 textures aren't valid in desktop GL until 3.0

Good news though: LUMINANCE/LUMINANCE/UNSIGNED_BYTE is valid. If we switch to Lum8 textures, we can unconditionally have our shaders use `.r` instead of `.a`.

Specifically, we should be using R8 textures on GL3.0+ and GLES3.0+, but LUMINANCE/LUMINANCE/UNSIGNED_BYTE otherwise.

There is the small outside chance of a bad driver implementing Luminance8 textures as RGB8 textures with duplicated values, and thus tripling our memory usage for Luminance8 textures, but we should cross that bridge only if we come to it.

::: gfx/gl/GLBlitHelper.cpp
@@ +614,3 @@
>      if (!tex) {
>          MOZ_ASSERT(needsAllocation);
> +        tex = CreateTexture(mGL, LOCAL_GL_RED, LOCAL_GL_RED, LOCAL_GL_UNSIGNED_BYTE,

This is not valid GLES3:
GLES 3.0.4 p112 tells us that if we use RED/UNSIGNED_BYTE for format/type, our internalFormat must be R8.

We should prefer R8 where available, which is GLES3+ and GL3.0+.
Attachment #8721884 - Flags: review?(jgilbert) → review-
I use different format by GL/EGL version and the shader is using r channel. But there is a small bug in the "IsAtLeast", so I also fixed it.
Attachment #8721884 - Attachment is obsolete: true
Attachment #8722416 - Flags: review?(jgilbert)
Attachment #8722416 - Flags: review?(jgilbert) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/33a9e4482bcc
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.