Bug 88905

There seems to be issues with texture upload when type is GL_FLOAT and format is GL_LUMINANCE, GL_ALPHA or GL_LUMINANCE_ALPHA.

(In reply to Tapani Pälli from comment #1)
> There seems to be issues with texture upload when type is GL_FLOAT and
> format is GL_LUMINANCE, GL_ALPHA or GL_LUMINANCE_ALPHA.

urgh unfortunately the problem is bigger than this, plan was to expose this for only ES2 but it seems ES3 also now uses it, this is not taken account in mesa_es3_error_check_format_and_type and perhaps some other places .. will dig deeper.

(In reply to Tapani Pälli from comment #2)
> (In reply to Tapani Pälli from comment #1)
> > There seems to be issues with texture upload when type is GL_FLOAT and
> > format is GL_LUMINANCE, GL_ALPHA or GL_LUMINANCE_ALPHA.
> 
> urgh unfortunately the problem is bigger than this, plan was to expose this
> for only ES2 but it seems ES3 also now uses it, this is not taken account in
> mesa_es3_error_check_format_and_type and perhaps some other places .. will
> dig deeper.

To be clear, I mean that some of the added half-float support seems to be enabling/executed also for ES3 which makes test fail.

(In reply to Tapani Pälli from comment #3)
> (In reply to Tapani Pälli from comment #2)
> > (In reply to Tapani Pälli from comment #1)
> > > There seems to be issues with texture upload when type is GL_FLOAT and
> > > format is GL_LUMINANCE, GL_ALPHA or GL_LUMINANCE_ALPHA.
> > 
> > urgh unfortunately the problem is bigger than this, plan was to expose this
> > for only ES2 but it seems ES3 also now uses it, this is not taken account in
> > mesa_es3_error_check_format_and_type and perhaps some other places .. will
> > dig deeper.
> 
> To be clear, I mean that some of the added half-float support seems to be
> enabling/executed also for ES3 which makes test fail.

And unfortunately the added float support too :/ I will investigate how to make this specific for gles2.

Created attachment 113254 [details] [review]
workaround

Here's a first workaround that makes the test pass. I will need to get more familiar with the texture and renderbuffer validation code to understand why this validation does not already happen.

Created attachment 113258 [details] [review]
workaround

workaround, real fix will be to correct validation code

(In reply to Tapani Pälli from comment #6)
> Created attachment 113258 [details] [review] [review]
> workaround
> 
> workaround, real fix will be to correct validation code

Fixed by this patch.
dEQP Core GL-CTS-2.0 (0x0052484b) starting..
  target implementation = 'X11'

Test case 'ES3-CTS.gtf.GL3Tests.packed_pixels.packed_pixels'..
INFO:packed_pixels:countReadPixels: 8821, countReadPixelsOK: 47
INFO:packed_pixels:countCompare: 47, countCompareOK: 47
  Pass (Pass)

DONE!

Test run totals:
  Passed:        1/1 (100.00%)
  Failed:        0/1 (0.00%)
  Not supported: 0/1 (0.00%)
  Warnings:      0/1 (0.00%)

Now I understand the problem. OES_texture_float does not specify floating point render target support. Our implementation has this because it is 'expected' by the corresponding WebGL extension. Related discussion in the khronos public bugzilla:

https://www.khronos.org/bugzilla/show_bug.cgi?id=729

There's also bunch of bugs in the internal bugzilla. WebGL extension specification mentions that new implementation should not implement render target support but implement WEBGL_color_buffer_float which maps with  EXT_color_buffer_half_float:

https://www.khronos.org/registry/webgl/extensions/OES_texture_float/

So, to put it short our implementation is wrong in allowing render targets to be created with GL_FLOAT and GL_HALF_FLOAT. Unfortunately fixing this means that we will loose some nice deferred rendering webgl demos enabled by current implementation, we could think of having override to allow this later (?) Will investigate a proper fix.

Created attachment 113391 [details] [review]
fix

set fbo incomplete if GL_FLOAT or GL_HALF_FLOAT is set as internalformat.

Lu Hua, could you run full suite with the patch to see any possible regressions?

fix pushed

(In reply to Tapani Pälli from comment #10)
> Lu Hua, could you run full suite with the patch to see any possible
> regressions?

Test full ogles3conform, this issue  is fixed by this patch and no new regression.
                                                           master   master with patch
ES3-CTS.gtf.GL3Tests.packed_pixels.packed_pixels           fail           pass
ES3-CTS.gtf.GL3Tests.packed_pixels.packed_pixels_pbo       fail           pass
ES3-CTS.gtf.GL3Tests.packed_pixels.packed_pixels_pixelstore fail           pass

(In reply to lu hua from comment #12)
> (In reply to Tapani Pälli from comment #10)
> > Lu Hua, could you run full suite with the patch to see any possible
> > regressions?
> 
> Test full ogles3conform, this issue  is fixed by this patch and no new
> regression.
>                                                            master   master
> with patch
> ES3-CTS.gtf.GL3Tests.packed_pixels.packed_pixels           fail          
> pass
> ES3-CTS.gtf.GL3Tests.packed_pixels.packed_pixels_pbo       fail          
> pass
> ES3-CTS.gtf.GL3Tests.packed_pixels.packed_pixels_pixelstore fail          
> pass

Nice, thanks for testing round!

Verified.Fixed.