greggman merges #4
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The original code setup to user properties on the WebGLRenderingContext call `gl.viewportWidth` and `gl.viewportHeight`. The issue with these are (1) they end up looking like part of the WebGL spec which is confusing for new users and (2) they are not live so they need to be updated if the canvas changes size. Instead I changed them to `gl.canvas.width` and `gl.canvas.height` where appropriate. Also for the perspective matrix I changed them to `gl.canvas.clientWidth` and `gl.canvas.clientHeight` which means they'll be correct even if the display size of the canvas does not match the size of the canvas' backing store. For example if the canvas is sized using CSS.
The examples were using
<meta http-equiv="content-type" content="text/html; charset=ISO-8859-1">
Which is out-dated. Any browser that supports WebGL supports the newer way.
It's also not very socially aware as it's English only.
Changed to
<meta charset="utf-8">
Which is the modern way as of like 6 years ago and is also internationally
friendly.
1. Got rid of setInterval which is considered bad practice as
it DOSes the user's machine
2. Initialized textured to avoid warning.
Because images are loaded asynchronously and the code does not wait
for the image to load it can start rendering before the image has been
downloaded. In that case a rendering warning appears.
[.WebGLRenderingContext]RENDER WARNING: texture bound to texture unit 0 is not renderable. It maybe non-power-of-2 and have incompatible texture filtering or is not 'texture complete'
Initializing the texture to a single pixel means the code and render with
no warnings and the texture will be uploaded if and when the image is
finally downloaded.
The problem with putting properties on WebGLObjects is your app
will crash if you ever decide to handle WebGLContextLost events.
The reason is all of the gl.createXXX functions will return null
and so code like this
var buffer = gl.createBuffer();
buffer.itemSize = 3;
Will crash because buffer will be null. The simplest solution
is to use JavaScript objects. So for example instead of
var buffer = gl.createBuffer();
buffer.itemSize = 3;
gl.bindBuffer(..., buffer);
We can change it to
var buffer = { id: gl.createBuffer(); };
buffer.itemSize = 3;
gl.bindBuffer(..., buffer.id);
That at least is future proof code.
Stop putting properties on WebGLObjects
Change to utf-8
Remove gl.viewportWidth and gl.viewportHeight
Conflicts: example03/particles-01-noenable.html example03/particles-01.html example03/particles-02.html example03/particles-03.html
All images load asynchronously which means either you should not start rendering until they are loaded or else you need to put something in the textures so you can render immediately. This patch initializes all textures to a 1x1 pixel blue texture. When the image finishes loading the code replaces that contents of that texture with the image. Until then it's free to render with the one pixel texture This gets rid of the rendering warnings from the browser in the JavaScript console.
Fix all image to texture loading to not generate warnings
|
This looks good. I've been learning/following the tutorials and upgrading gl-matrix in each, but this PR makes many more improvements. It seems like the tutorials would need to be updated as well though - perhaps that could be a group effort if the HTML for those are available to be posted? Also - I noticed that, after upgrading gl-matrix to the latest version, the Z-axis translations are much further away than in the original examples. Is that an issue with the old version of gl-matrix? |
Owner
|
Hi Brian, Not sure about the Z-axis issues. I will definitely be posting something to the site soon about the upgrade. I was planning on editing the HTML myself but that's a great idea to crowdsource it! I'm slammed this week with conferences but we should talk about this next week. Where are you located? Tony |
|
@tparisi I'm currently in the North East US - let me know when you're thinking for next week and we can set it up. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
No description provided.