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

Automated tests #21

Closed
jbuck opened this issue Sep 16, 2011 · 9 comments

Comments

@jbuck
Copy link
Contributor

commented Sep 16, 2011

Rather than running through the manual tests in the tests directory, devs should be able to open a page in their browser and have it run a bunch of reference tests to make sure that a patch doesn't cause a regression.

@Anachid

This comment has been minimized.

Copy link
Contributor

commented Sep 21, 2011

Hello, I am currently looking at the possibility of Automating the performance test and check for how fast it takes to run. If possible could you please assign this issue to me. I can work on it.

@ghost ghost assigned humphd Sep 21, 2011

@cjcliffe

This comment has been minimized.

Copy link
Owner

commented Sep 21, 2011

I don't think I can assign this directly without giving you read/write access to the master repo -- but I've assigned it to humphd which can be used to reference class-assigned issues. Make your own fork and you should be able to file your own issues and issue pull requests when they've been completed.

@cjcliffe

This comment has been minimized.

Copy link
Owner

commented Nov 18, 2011

Took a few runs through the performance test page -- looking great so far; should be easy to add more tests.

I did notice that the sphere map test doesn't seem to make it through a second run here -- gives me:

WebGL: bindTexture: object from different WebGL context (or older generation of this one) passed as argument @ file:///home/ccliffe/Dev/gitCubicVR/source/CubicVR.Texture.js:182

@Anachid

This comment has been minimized.

Copy link
Contributor

commented Nov 18, 2011

Took a look into them...It seemed I was creating a SphereMesh using the old way.

var sphereMesh = new CubicVR.Mesh({
primitive: {
.....
textures: {
color: "../images/2576-diffuse.jpg",
normal: "../images/2576-normal.jpg",
bump: "../images/2576-bump.jpg",
envsphere: "../images/fract_reflections.jpg"
}
...
});

When I should have done this:

textures: {
color: new CubicVR.Texture("../images/2576-diffuse.jpg"),
normal: new CubicVR.Texture("../images/2576-normal.jpg"),
bump: new CubicVR.Texture("../images/2576-bump.jpg"),
envsphere: new CubicVR.Texture("../images/fract_reflections.jpg")
}

I'll push a pull request to you soon.

@cjcliffe

This comment has been minimized.

Copy link
Owner

commented Nov 18, 2011

Aha -- I'm sure that's not strictly an issue with the test.. You're technically reloading a new copy of those each time by doing the 'new Texture' which isn't technically necessary and could be wasting some memory -- I'll have to take a look at adding a function to let you flush the texture memory on GPU without having to reload the ones in main memory for a quick reload -- similarly we should probably have a way to flush the main texture memory as well..

Ultimately this is still an issue with changing contexts which we need to address as a bigger problem...

For now a 'new Texture' call will be cool, we'll just let it leak a bit and GC when done -- as an alternative I don't think it would be unreasonable to render them all to the same WebGL canvas without re-initialization and if desired snapshot a copy of the last frame of each run to place beside the benchmark result step.

@Anachid

This comment has been minimized.

Copy link
Contributor

commented Nov 18, 2011

well, we can open up a new issue for the Texture flush (useful for tracking purpose)

As for the alternative. I could look into rendering into the same WebGL canvas and keep the snapshot of the last frame. That also sounds more efficient, performance-wise.

@cjcliffe

This comment has been minimized.

Copy link
Owner

commented Nov 18, 2011

I think we can leave it as possibly a side note for Issue #37 (multi-canvas-support) for the texture flushing as it's part of that.

If you want to try for the single-canvas approach I can leave this ticket open or I can close it and you can submit a new one for that process.

@Anachid

This comment has been minimized.

Copy link
Contributor

commented Nov 18, 2011

Leave it open for a little longer. I am currently trying to allow a user change settings on each run of MainLoop. When I get that working, most of the test framework should be in place and we can close this issue then. I'll try to get the single-canvas approach along as well.

@cjcliffe

This comment has been minimized.

Copy link
Owner

commented Dec 12, 2011

closing issue as pull request has been merged

@cjcliffe cjcliffe closed this Dec 12, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.