Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update EMSCRIPTEN_WEBGL_CONTEXT_HANDLE to cover full pointer range #21281

Merged
merged 1 commit into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -819,6 +819,7 @@ jobs:
browser_2gb.test_emscripten_animate_canvas_element_size_manual_css
browser_2gb.test_fulles2_sdlproc
browser_2gb.test_cubegeom*
browser_2gb.test_html5_webgl_create_context*
"
test-browser-chrome-wasm64-4gb:
executor: bionic
Expand All @@ -834,6 +835,7 @@ jobs:
browser64_4gb.test_fetch*
browser64_4gb.test_emscripten_animate_canvas_element_size_manual_css
browser64_4gb.test_fulles2_sdlproc
browser64_4gb.test_html5_webgl_create_context*
"
test-browser-firefox:
executor: bionic
Expand Down
6 changes: 6 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ See docs/process.md for more on how version tagging works.

3.1.54 (in development)
-----------------------
- The type of `EMSCRIPTEN_WEBGL_CONTEXT_HANDLE` was changed to unsigned and
the only valid error returned from `emscripten_webgl_create_context` is
now zero. This allows `EMSCRIPTEN_WEBGL_CONTEXT_HANDLE` to hold a pointer
to memory even in 2GB+ mode. Since `emscripten_webgl_create_context` never
returns anything except zero for its errors today this change should not
require any action. (#21268)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm interesting.. Originally the intent was that it would have been returning the same set of negative return codes, i.e.

#define EMSCRIPTEN_RESULT_NOT_SUPPORTED -1
#define EMSCRIPTEN_RESULT_FAILED_NOT_DEFERRED -2
#define EMSCRIPTEN_RESULT_INVALID_TARGET -3
#define EMSCRIPTEN_RESULT_UNKNOWN_TARGET -4
#define EMSCRIPTEN_RESULT_INVALID_PARAM -5
#define EMSCRIPTEN_RESULT_FAILED -6
#define EMSCRIPTEN_RESULT_NO_DATA -7
#define EMSCRIPTEN_RESULT_TIMED_OUT -8
to allow differentiating between "WebGL is not supported" and "you just inputted a selector to a DOM element that doesn't exist", but looks like indeed that distinction doesn't exist (or maybe has been lost in a refactor)

LGTM overall, looks like debug builds have prints that mention which kind of failure it was, so I think we'll go with those.

- Added `--use-port` option to `emcc`. This option allows ports to be enabled
by name and is designed to replace all existing `-sUSE_XXX` settings for
ports. You can use `--show-ports` to get the list of available ports that
Expand Down
2 changes: 1 addition & 1 deletion site/source/docs/api_reference/html5.h.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2086,7 +2086,7 @@ Functions
:type target: const char*
:param attributes: The attributes of the requested context version.
:type attributes: const EmscriptenWebGLContextAttributes*
:returns: On success, a strictly positive value that represents a handle to the created context. On failure, a negative number that can be cast to an |EMSCRIPTEN_RESULT| field to get the reason why the context creation failed.
:returns: On success, a non-zero value that represents a handle to the created context. On failure, 0.
:rtype: |EMSCRIPTEN_WEBGL_CONTEXT_HANDLE|


Expand Down
2 changes: 1 addition & 1 deletion system/include/emscripten/html5_webgl.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
extern "C" {
#endif

typedef intptr_t EMSCRIPTEN_WEBGL_CONTEXT_HANDLE;
typedef uintptr_t EMSCRIPTEN_WEBGL_CONTEXT_HANDLE;
kainino0x marked this conversation as resolved.
Show resolved Hide resolved

typedef int EMSCRIPTEN_WEBGL_CONTEXT_PROXY_MODE;
#define EMSCRIPTEN_WEBGL_CONTEXT_PROXY_DISALLOW 0
Expand Down
Loading