Bug 33959 - Don't call vgGetError()/eglGetError() repeatedly
Summary: Don't call vgGetError()/eglGetError() repeatedly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 33405 33987
  Show dependency tree / graph
 
Reported: 2010-01-21 10:45 PST by Jakob Petsovits
Modified: 2010-01-21 20:44 PST (History)
2 users (show)

See Also:

Attachments
Patch (9.50 KB, patch)
2010-01-21 10:48 PST, Jakob Petsovits 		manyoso: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jakob Petsovits 2010-01-21 10:45:56 PST 
Don't call vgGetError()/eglGetError() repeatedly
Comment 1 Jakob Petsovits 2010-01-21 10:48:16 PST 
Created attachment 47130 [details]
Patch
Comment 2 Jakob Petsovits 2010-01-21 10:58:23 PST 
While changing the VG/EGL error macros to something that reviewer Niko liked better, I introduced a bug in there. I thought I could be smart and get rid of the temporary variable that holds the error code, and tested it with eglGetError() on the OpenVG reference implementation to make sure it works.

Turns out that wasn't the best idea. On the one hand, vgGetError() turns out to be specified differently than eglGetError() so it must only be called once in any case. On the other hand, eglGetError() turns out to be so vaguely specified that implementations and even EGL patron Khronos interprets the spec inconsistently.

On a more positive note, if I hadn't introduced the bug in the first place, the SurfaceOpenVG pbuffer constructor (and the related EGLDisplayOpenVG::createPbufferSurface()) would have suffered from a subtle bug which is being taken care of in this patch as well.

Please read the commit message too, and review. Thanks!
Comment 3 Adam Treat 2010-01-21 12:10:13 PST 
Comment on attachment 47130 [details]
Patch

As discussed on irc, r=me with the removal of unused 'ASSERT_VG_NO_ERROR_VALUE'
Comment 4 Jakob Petsovits 2010-01-21 12:56:49 PST 
Committed r53639: <http://trac.webkit.org/changeset/53639>