Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Make egui_glow painter to work on web #868
Make egui_glow painter to work on web #868
Changes from 15 commits
61cc0dc
fa22f04
33648f7
2c3296e
eaf58a0
4eb841e
d71656e
e3b4120
1887768
901cfa0
5bd0d5d
88027b1
fb42a66
51981c5
18b05b9
dd2f8a6
a4d7146
09c32b6
a47e7e5
ff25929
22245be
dc72728
e95ecb6
7faa359
7ac5c6a
a9c8b26
efcadb5
58ab9e6
c496e92
385c1fb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is nitpicky, but I think it would make sense to offer two constructors here instead of one.
Also, wouldn't the API be nicer if the constructor captured the
&glow::Context
here so it doesn't need to be passed to the other methods? That might also help reduce some confusion about whyVAO::bind_vertex_array
is unsafe, but the others are not?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point about two constructors 👍
Capturing
&glow::Context
leads to a lot of ugly lifetime issues. Passing it in by reference is also more explicit, i.e. easier to see what is actually doing graphics calls. This was discussed in the original glow PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok i will split constructor.
but is there good way to switch emulated mode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok! Good to know, I didn't follow that conversation closely.
This question implies there is a way to switch the emulation mode with just one constructor? I don't believe that is the case before or after the suggested patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought this procedure good enough to switch emulated mode .
Is there any suggestion?