search

WebGLSamples / WebGL2Samples

Short and easy to understand samples demonstrating WebGL 2 features
Other
1.01k stars 143 forks source link

Transform feedback separated 2 #86

Closed trungtle closed 8 years ago

trungtle commented 8 years ago

Particle sym transform feedback. @pjcozzi I removed the self-executing pattern in init program, but wonder if this is clean enough.

pjcozzi commented 8 years ago

@kenrussell do you want to look at this before we merge?

kenrussell commented 8 years ago

Looks good. One comment.

shrekshao commented 8 years ago

So basically all deconstructings are unnecessary in webgl or just in chrome? Thanks for pointing thIs out

On Saturday, February 13, 2016, Ken Russell notifications@github.com wrote:

Looks good. One comment.

— Reply to this email directly or view it on GitHub https://github.com/WebGLSamples/WebGL2Samples/pull/86#issuecomment-183689326 .

kenrussell commented 8 years ago

Cleanup of WebGL resources upon page unload is not necessary in any browser.

However, if you have a long-running page that allocates a lot of resources, it's very important to explicitly delete the ones you are no longer using, rather than letting the browser's garbage collector clean them up.

On Sat, Feb 13, 2016 at 7:46 AM, Shuai Shao (Shrek) < notifications@github.com> wrote:

So basically all deconstructings are unnecessary in webgl or just in chrome? Thanks for pointing thIs out

On Saturday, February 13, 2016, Ken Russell notifications@github.com wrote:

Looks good. One comment.

— Reply to this email directly or view it on GitHub < https://github.com/WebGLSamples/WebGL2Samples/pull/86#issuecomment-183689326

.

— Reply to this email directly or view it on GitHub https://github.com/WebGLSamples/WebGL2Samples/pull/86#issuecomment-183690306 .

trungtle commented 8 years ago

Thanks @pjcozzi and @kenrussell for the feedback. I removed the onunload cleanup code and updated u_time properly now. Should be good to go.

pjcozzi commented 8 years ago

Most real WebGL apps are long-running pages that need to do this sort of cleanup, and we want to encourage property memory management. With that said, if it is a bad practice for this case with the onunload event (as opposed to many of our other samples that only render the frame once and can delete resources right away), then perhaps we could leave the delete code in, but commented out and not in the onunload? I'm open to other ideas, but proper resource management is too important to just completely leave out.

trungtle commented 8 years ago

We could:

  1. Leave the cleanup codes in with a big comment on why or
  2. I could have the sample runs for a certain amount of time (let's say 60 seconds) then stops and cleans up (add button to replay). With the latter case, the code for deleting resources can be included with correct intention (unless page refresh doesn't clean up).

What I don't know is if we can rely on page refresh or page navigation across all browsers to delete resources.

pjcozzi commented 8 years ago

2 is a workaround. 1 still sort of promotes the onunload bad practice. Why not leave just the delete code commented out at the end with a comment that says something like "For explicit WebGL resource cleanup for long-running pages:"

trungtle commented 8 years ago

It sounds good to me! Updated, sorry so late to update this.

kenrussell commented 8 years ago

Looks good to me otherwise.

pjcozzi commented 8 years ago

@trungtle let me know when you have the final wording for this.

trungtle commented 8 years ago

@pjcozzi Updated the comment.

pjcozzi commented 8 years ago

I cleaned it up more.