-
Notifications
You must be signed in to change notification settings - Fork 20
WebGL section initial content. Updated the examples. #155
Conversation
|
|
@kenchris please review. |
index.html
Outdated
@@ -1194,16 +1197,102 @@ | |||
This section is currently work in progress, and subject to change. | |||
</div> | |||
<p> | |||
Use cases like rendering 3D point cloud, background removal, |
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 would do
There are several use-cases which are a good fit to be, at least partially, implemented on the GPU, such as motion recognition, pattern recognition, background removal, as well as 3D point cloud.
This section explains which APIs can be used for some of these mentioned use-cases; ...
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.
thanks
index.html
Outdated
texture of format <code>RGB</code> and type | ||
<code>UNSIGNED_BYTE</code>. [[WEBGL]] | ||
texture of format <code>RGBA</code> or <code>RED</code> and type | ||
<code>FLOAT</code>. See also the specification [[WEBGL]] and the |
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.
See also sounds a bit weird to me. Maybe just remove also?
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.
thanks
index.html
Outdated
"mjx-vsize"></span></span></span></span></span></span></span> | ||
</p> | ||
<h3> | ||
Read the data from WebGL texture |
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.
from a
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.
thanks
index.html
Outdated
Read the data from WebGL texture | ||
</h3> | ||
<p> | ||
We list here some of the possible approaches:In addition to , there |
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.
Here we list some of the possible approaches. (s/:/. and remove space before ,)
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.
in addition to what?
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.
thanks. should have remove it. Now the content is only:
Here we list some of the possible approaches.
index.html
Outdated
are two ways available for asynchronous access: | ||
</p> | ||
<ul> | ||
<li>Synchronous readPixels requires the least amount of code and it |
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.
You said async above and now talk about sync...
Do you mean: In addition to asynchronous access, there are ....
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.
removed the mention of async above.
index.html
Outdated
<ul> | ||
<li>Synchronous readPixels requires the least amount of code and it | ||
is available om WebGL1. See the <a>readPixels from float</a> | ||
example for details. |
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.
for further details?
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.
thanks.
index.html
Outdated
example for details. | ||
</li> | ||
<li>Asynchronous readPixels using pixel buffer objects should | ||
remove the blocking while waiting for the readPixels call. |
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.
the -> any blocking?
waiting for the readPixels call to finish?
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.
Replaced by: Asynchronous readPixels using pixel buffer objects to avoid blocking the readPixels call.
index.html
Outdated
remove the blocking while waiting for the readPixels call. | ||
</li> | ||
<li>Transform feedback with GetBufferSubData(Async) provides | ||
synchronos and asynchronous read access to depth and color texture |
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.
spellign of synchronous
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.
thanks
index.html
Outdated
</li> | ||
<li>Transform feedback with GetBufferSubData(Async) provides | ||
synchronos and asynchronous read access to depth and color texture | ||
data processed in vertex shader. |
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.
in the vertex shader
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.
thanks.
</ul> | ||
<p></p> | ||
<p class="note"> | ||
Performance of synchronous <a>readPixels from float</a> example in |
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.
a bit hard to read.
The "readPixel from float" example above, while being simplistic, suffers from some performance issues, due to the fact that ... This can be mitigated by...
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 wanted to say: blocking is not bad, it is only 3-6 ms on my development laptops.
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.
But web vr and such use cases will be on entirely different hardware, so not sure that we can make such a statement
index.html
Outdated
// texture, and samples that texture in the fragment shader, reconstructing the | ||
// 16-bit depth values from the red and green channels. | ||
// This code sets up a video element from a depth stream, uploads it to a WebGL2 | ||
// float texture holding the full precision data as the 16-bit depth map. |
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.
holding -> containing?
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.
thx
index.html
Outdated
@@ -1256,31 +1344,78 @@ | |||
// handle gUM error here | |||
}); | |||
|
|||
// ... initialize WebGL2 context when application starts. |
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.
pretty useless comment, if people dont know that, they cannot understand any example here :) it is basic knowledge of webgl. Comments should make sense, or dont have them
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, removed
index.html
Outdated
This example extends <a>upload to float texture</a> example. | ||
</p> | ||
<pre class="example"> | ||
// This code sets up a named framebuffer, attach the texture we upload the depth |
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.
attach the texture we upload the depth - sounds weird
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.
Changed my mind about: "the texture we upload the depth video to" -> "the texture with depth data". Doesn't have the depth data when created.
Now:
// This code creates the texture to which we will upload the depth video frame.
// Then, it sets up a named framebuffer, attach the texture as color attachment
index.html
Outdated
</p> | ||
<pre class="example"> | ||
// This code sets up a named framebuffer, attach the texture we upload the depth | ||
// video to as color attachment and, in rendering time, reads the texture |
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.
AT rendering time
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.
thanks. all replaced by:
// This code creates the texture to which we will upload the depth video frame.
// Then, it sets up a named framebuffer, attach the texture as color attachment
// and, after uploading the depth video to the texture, reads the texture
// content to Float32Array.
index.html
Outdated
// video to as color attachment and, in rendering time, reads the texture | ||
// content to Float32Array. | ||
|
||
// ... initialize texture and framebuffer for reading back the texture... |
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.
why the ... at the beginning, what purpose do they serve?
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.
Don't have a clue. It was there before.
I am often comfortable accepting existing code approach and replicating it without understanding what is it for.
index.html
Outdated
gl.FLOAT, | ||
depthVideo); | ||
|
||
var buffer = new Float32Array(depthVideo.videoWidth * depthVideo.videoHeight); |
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.
const should work instead of var here... var is not recommended in JS now. const doesnt make the object const, only the reference which cannot be reassigned.
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. It is better to pre-allocate the buffer outside rendering loop, and we can do it only after texImage2D and gl.getParameter(gl.IMPLEMENTATION_COLOR_READ_FORMAT) call.
That's why it is var declared somewhere else but initialized here.
It complicates the example so I will use const and put the comment.
@kenchris |
index.html
Outdated
mentioned use-cases; the concrete usage examples are provided in | ||
the <a href= | ||
"https://www.w3.org/wiki/Media_Capture_Depth_Stream_Extension#Examples"> | ||
Examples</a> section. |
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.
section below?
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.
copying approach from other places, prefer not to have bellow together with link.
index.html
Outdated
Examples</a> section. | ||
</p> | ||
<h3> | ||
Upload video frame to WebGL texture |
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.
a video frame, or video frames?
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.
video frames, since there is a mention of loop. thanks.
A <a>video</a> element whose source is a <a>MediaStream</a> object | ||
containing a <a>depth stream track</a> may be uploaded to a WebGL | ||
texture of format <code>RGB</code> and type | ||
<code>UNSIGNED_BYTE</code>. [[WEBGL]] | ||
texture of format <code>RGBA</code> or <code>RED</code> and type |
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.
Not BLUE? If not, explain why. if yes, then change the wording
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.
There is no BLUE texture in webgl.
index.html
Outdated
"mjx-vsize"></span></span></span></span></span></span></span> | ||
</p> | ||
<h3> | ||
Read the data from a WebGL texture |
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 am wondering whether people say "of a texture" - so probably check with a native speaker
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.
from a texture memory, from a texture or read back the texture data is used. keeping the existing.
index.html
Outdated
Here we list some of the possible approaches. | ||
</p> | ||
<ul> | ||
<li>Synchronous readPixels requires the least amount of code and it |
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.
Synchronous calls to readPixels
, which requires...
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.
Synchronous readPixels usage. It is the same call as async, just different setup.
index.html
Outdated
</p> | ||
<ul> | ||
<li>Synchronous readPixels requires the least amount of code and it | ||
is available om WebGL1. See the <a>readPixels from float</a> |
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.
om? works with WebGL 1.0 (that is the spec name :))
readPixels should be with right?
index.html
Outdated
<li>Asynchronous readPixels using pixel buffer objects to avoid | ||
blocking the readPixels call. | ||
</li> | ||
<li>Transform feedback with GetBufferSubData(Async) provides |
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 am not sure it is clear what "transform feedback" is, or whether it is one concept... can we add link or at least make it cursive or so
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.
Links to khronos specifications added for both Transform feedback with GetBufferSubDatasync
Don't have a clue how to add informative reference to the bottom of the page [WEBGL2], @anssiko how to do that? Thanks
index.html
Outdated
@@ -1239,12 +1330,11 @@ | |||
); | |||
</pre> | |||
<h3> | |||
WebGL Fragment Shader based post-processing | |||
WebGL2: <dfn>upload to float texture</dfn> |
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.
Spec is called WebGL 2.0
index.html
Outdated
// texture, and samples that texture in the fragment shader, reconstructing the | ||
// 16-bit depth values from the red and green channels. | ||
// This code sets up a video element from a depth stream, uploads it to a WebGL2 | ||
// float texture containing the full precision data as the 16-bit depth map. |
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.
How is this depth map exposed, Uint16Array?
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.
removed the comment.
depth map is 16 bit. It is not exposed.
The texture is float.
index.html
Outdated
@@ -1256,31 +1346,80 @@ | |||
// handle gUM error here | |||
}); | |||
|
|||
// ... later, in the rendering loop ... | |||
var gl = canvas.getContext("webgl2"); | |||
// Activate the extension to use single component R32F texture format. |
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.
Is that alwasy available in WebGL 2.0? if so I would write
Activate the standard WebGL 2.0 extension for using single component R32F texture format
index.html
Outdated
// Activate the extension to use single component R32F texture format. | ||
gl.getExtension('EXT_color_buffer_float'); | ||
|
||
// Later, in the rendering loop ... |
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.
Could be have this be in another box in the spec, so it is super clear that this is a separate part of the code
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.
It is better if handled in your PR #154.
index.html
Outdated
</pre> | ||
<h3> | ||
WebGL2: <dfn>readPixels from float</dfn> texture |
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.
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.
Replaced WebGL2 by WebGL.
index.html
Outdated
This example extends <a>upload to float texture</a> example. | ||
</p> | ||
<pre class="example"> | ||
// This code creates the texture to which we will upload the depth video frame. |
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 should not be in the code comments... check my former patch to see how it can be done as part of the example box
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.
moved out to
. You are adding in PR #154 so it is better to handle it there for all. Thanks
index.html
Outdated
// and, after uploading the depth video to the texture, reads the texture | ||
// content to Float32Array. | ||
|
||
// Initialize texture and framebuffer for reading back the texture... |
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.
Sometimes it is better to define functions with descriptive names instead of comments. It also makes the code nicely scoped and easy to read
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.
Please feel free to handle it in your PR #154. My goal with this patch is to replace the original example code - since it is not supported and need to be removed ASAP. Cosmetic changes will need to be handled in follow up.
index.html
Outdated
// content to Float32Array. | ||
|
||
// Initialize texture and framebuffer for reading back the texture... | ||
var depthTexture = gl.createTexture(); |
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.
const
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.
let
index.html
Outdated
gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MAG_FILTER, gl.NEAREST); | ||
gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MIN_FILTER, gl.NEAREST); | ||
|
||
var framebuffer = gl.createFramebuffer(); |
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.
const
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.
let - it is supposed to be used in different scope from creation. feel free to refactor in your patch
index.html
Outdated
gl.FLOAT, | ||
depthVideo); | ||
|
||
// It is recommended to allocate this buffer once. |
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.
only once? I dont think the comment is clear
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.
replaced by:
if (!buffer) {
buffer =
new Float32Array(depthVideo.videoWidth * depthVideo.videoHeight);
}
PR updated. Open: |
Can you rebase this so it can get merged? |
Rebased. Thanks. |
@astojilj You can add your Khronos informative references to To make a reference informative in the prose, use |
Renoves the splitting depth to red and green component.
@anssiko I will start adding [WEBGL2] and [WEBGL-GET-BUFFER-SUB-DATA-ASYNC] to SPECREF |
Thanks! |
Renoves the splitting depth to red and green component.
@anssiko @huningxin PTAL