Skip to content

Remove jsevents object#3298

Closed
juj wants to merge 19 commits intoemscripten-core:incomingfrom
juj:remove_jsevents_object
Closed

Remove jsevents object#3298
juj wants to merge 19 commits intoemscripten-core:incomingfrom
juj:remove_jsevents_object

Conversation

@juj
Copy link
Collaborator

@juj juj commented Apr 1, 2015

This pull request removes the JSEvents object altogether in src/library_html5.js and replaces it with the use of explict x__deps dependency handling. This allows dead code elimination to remove a lot of unused functions when using the HTML5 events API. Also the functions are now prefixed with double underscores to clearly separate public and private APIs. This fixes #2999.

juj added 19 commits April 1, 2015 17:08
… x__deps more to get better dead code elimination. Fixes emscripten-core#2999.
…SEvents object for better dead code elimination opportunities. emscripten-core#2999.
@juj juj added the HTML5 API label Apr 1, 2015
@juj
Copy link
Collaborator Author

juj commented Apr 1, 2015

The intent of this is also to prepare for fixing issue #3283 by changing the built-in default shell.html to reuse the HTML5 functions so that the fullscreen button and the HTML5 API fullscreen request code will play nice together. However I'm not sure yet how to model a dependency between default shell.html to _emscripten_request_fullscreen. Would you have an idea for this @kripken ?

@kripken
Copy link
Member

kripken commented Apr 1, 2015

I think if an HTML shell file depends on something, it would need to add it as used by adding it to DEFAULT_LIBRARY_FUNCS_TO_INCLUDE.

@kripken
Copy link
Member

kripken commented Apr 1, 2015

This is a lot of refactoring that leads to what I worry is worse code - now there will be lots of small globals (_currentEventHandler, _eventHandlers, etc.) as well as functions. It's all flattened out in the global namespace, with no obvious structure, unlike now. Is this really worth it to allow dependency management to remove some of this code?

If we are sure that the code size benefits are worth it, how about if there is still a $JSEvents, but it starts out almost empty, and at runtime we add properties to it when needed? Not sure if that would work or not.

Another option is to rely on closure. Closure advanced can remove parts of objects, so it would do dead code elimination in the code before this pull.

Another option is to add dependency management in emscripten to properties of objects, but that sounds complex.

@juj juj mentioned this pull request Oct 1, 2015
juj referenced this pull request Nov 13, 2017
…y(), emscripten_enter_soft_fullscreen() and emscripten_exit_soft_fullscreen() which allow a more intelligent switch between fullscreen modes with different preset display modes. The original emscripten_request_fullscreen() is still available, and should be used in scenarios where it's desired that the html5.h layer should perform no changes whatsoever to the DOM tree.
@kripken
Copy link
Member

kripken commented Nov 21, 2018

Is this still important, @juj?

@juj
Copy link
Collaborator Author

juj commented Nov 21, 2018

This is not only important, but critical; though this PR that implements this removal has fallen out of date, so I'll close this out. In my fork november_2018 I've reimplemented this, but don't have time just now to submit a PR.

@juj juj closed this Nov 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Moving emscripten_webgl_* out of library_html5.js?

2 participants