After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 669368 - Reading back texture fails with "Invalid operation" + wrong image
Reading back texture fails with "Invalid operation" + wrong image
Status: RESOLVED FIXED
Product: cogl
Classification: Platform
Component: CoglTexture
git master
Other Linux
: Normal normal
: ---
Assigned To: Cogl maintainer(s)
Cogl maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2012-02-04 16:59 UTC by drago01
Modified: 2012-02-07 15:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
cogl-texture: Fix error handling in get_texture_bits_via_offscreen (1.66 KB, patch)
2012-02-05 23:15 UTC, drago01
none Details | Review
I think it would be cleaner if the code just directly calls (3.52 KB, patch)
2012-02-06 12:27 UTC, Neil Roberts
reviewed Details | Review

Description drago01 2012-02-04 16:59:29 UTC
When trying to readback a texture (of a window) using cogl_texture_get_data in mutter I get:

Cogl-WARNING **: ./cogl-framebuffer.c:1313: GL error (1282): Invalid operation

And an image looking like this: http://193.200.113.196/files/bug.png

So it fails to bind the offscreen framebuffer and just continues reading from the onscreen buffer resulting into a wrong image.
Comment 1 drago01 2012-02-05 21:11:46 UTC
So I have added some debug prints to debug this and found that:

1) It hits to the get_texture_bits_via_offscreen path
2) It fails to create an FBO (failing with GL_FRAMEBUFFER_UNSUPPORTED)
3) There is zero error handling for the fbo creating (the return value as well as the gerror from cogl_framebuffer_allocate is ignored) so we fail do detect that this code path failed and just go on.
4) Forcing get_texture_bits_via_offscreen to always return FALSE fixes it (by skipping the code non working path).

This is on a NVIDIA 285 GTX using the propriety driver.
Comment 2 drago01 2012-02-05 23:15:09 UTC
Created attachment 206866 [details] [review]
cogl-texture: Fix error handling in get_texture_bits_via_offscreen

The return value of cogl_framebuffer_allocate is basically never checked
and in the places where it gets called there it cannot propagate the error
upwards to the callers, but we can detect a failed allocation by checking the
framebuffer's allocated flag.

So do that in get_texture_bits_via_offscreen to detect errors and fallback
to get_texture_bits_via_copy when the framebuffer allocation fails.

Without this the error never gets noticed and we end up reading from the
default framebuffer resulting into wrong content being read back.
Comment 3 Neil Roberts 2012-02-06 12:27:28 UTC
Created attachment 206876 [details] [review]
I think it would be cleaner if the code just directly calls

cogl_framebuffer_allocate itself to try and catch the error. It's safe
to call this function multiple times.

Here is another version of the patch to do that and also to add
similar checks to _cogl_blit_begin so that it can also fallback during
atlas texture reorganization.
Comment 4 drago01 2012-02-06 12:36:18 UTC
Review of attachment 206876 [details] [review]:

Yeah that makes sense to me and obviously fixes this bug just fine as well.
Comment 5 drago01 2012-02-06 12:36:41 UTC
Review of attachment 206876 [details] [review]:

Wrong review flag.
Comment 6 Neil Roberts 2012-02-07 15:25:10 UTC
Pushed to master as ec1bb4ce3 and to the 1.8 branch as d959af4. Thanks for looking into this.