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

Use EmscriptenApplication for emscripten ports #59

Closed
wants to merge 2 commits into from

Conversation

Squareys
Copy link
Contributor

@Squareys Squareys commented May 5, 2019

Hello @mosra !

As part of mosra/magnum#300, here are the adaptions to the emscripten example ports.

Cheers, Jonathan

@Squareys Squareys force-pushed the emscripten-application branch from ce1c86a to 4c328dc Compare May 5, 2019 18:10
@Squareys Squareys mentioned this pull request May 5, 2019
24 tasks
@Squareys Squareys force-pushed the emscripten-application branch from 4c328dc to 0545d02 Compare May 7, 2019 09:21
@Squareys
Copy link
Contributor Author

Squareys commented May 7, 2019

Note: With the current state, Chrome tabs crash on text example when loading the font (but works on firefox).

(obsolete)

Copy link
Owner

@mosra mosra left a comment

Choose a reason for hiding this comment

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

Just minor things, the tabindex comment from #60 applies here as well.

Note: With the current state, Chrome tabs crash on text example when loading the font (but works on firefox).

Is that still a thing or not anymore? I remember you said it's gone.

src/triangle/TriangleExample.cpp Outdated Show resolved Hide resolved
src/imgui/ImGuiExample.cpp Outdated Show resolved Hide resolved
@mosra mosra added this to the 2019.0b milestone May 17, 2019
@mosra mosra self-assigned this May 17, 2019
@Squareys Squareys force-pushed the emscripten-application branch 2 times, most recently from afbad9e to c9cd997 Compare May 22, 2019 10:31
@Squareys
Copy link
Contributor Author

Is that still a thing or not anymore? I remember you said it's gone.

Yeah, that's gone :)

@Squareys
Copy link
Contributor Author

tabindex comment from #60 applies here as well.

I'll just drop 76b75bc then 👍

Squareys added 2 commits May 23, 2019 09:52
Signed-off-by: Squareys <squareys@googlemail.com>
Signed-off-by: Squareys <squareys@googlemail.com>
@Squareys Squareys force-pushed the emscripten-application branch from c9cd997 to 4a21a21 Compare May 23, 2019 07:52
@Squareys
Copy link
Contributor Author

@mosra This is again ready for review!

Copy link
Owner

@mosra mosra left a comment

Choose a reason for hiding this comment

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

Looks good!

modules/FindMagnum.cmake Show resolved Hide resolved
@mosra
Copy link
Owner

mosra commented Jun 6, 2019

Merged as 3fb7bd8, with the imgui change being put directly into master as 4fae2fd. All the examples are now shipped on https://magnum.graphics/showcase/ and looking & behaving better than ever :)

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

2 participants