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

Depth mapped volume style #540

Merged
merged 7 commits into from Oct 21, 2015

Conversation

Projects
None yet
5 participants
@ankit1089

ankit1089 commented Aug 25, 2015

The DepthMappedVolumeStyle generates a volume rendering based on ray casting.
The rays terminate when they encounter a polygonal object inside the volume. Depth upto
which a particular ray can travel inside the volume is calculated from the depth buffer
of the polygonal objects in the volume.
The node also provides a method for generating flat cross-sectional views of the volume
at arbitrary orientations.

" vec3 cam_pos = vec3(modelViewMatrixInverse[3][0], modelViewMatrixInverse[3][1], modelViewMatrixInverse[3][2]);\n"+
" cam_pos = cam_pos/dimensions+0.5;\n"+

" const float steps = 200.0;\n"+ // take care! ANGLE does loop unrolling, setting this too high might not give any compile error but could give weird results

This comment has been minimized.

@aarbelaiz

aarbelaiz Aug 27, 2015

Contributor

Could this be "60.0" steps, in order to be consistent with the other volume rendering nodes?

rb = gl.createRenderbuffer();

gl.bindRenderbuffer(gl.RENDERBUFFER, rb);
gl.renderbufferStorage(gl.RENDERBUFFER, gl.DEPTH_COMPONENT, w, h);

This comment has been minimized.

@aarbelaiz

aarbelaiz Aug 27, 2015

Contributor

Check if there may be a typo: gl.DEPTH_COMPONENT16 was being used before.

Bug fix: Changed gl.DEPTH_COMPONENT to gl.DEPTH_COMPONENT16 in x3dom.…
…Utils.initFBO.

Modified DepthMappedVolumeStyle to use alpha value of transfer function for composting.
@ankit1089

This comment has been minimized.

ankit1089 commented Aug 31, 2015

@aarbelaiz Hi, thank you for your comments and pointing out the typo.
To be consistent with the other volume rendering nodes, I have changed the number of steps taken in the ray caster to 60.

@mlimper

This comment has been minimized.

Contributor

mlimper commented Aug 31, 2015

Thanks for this great contribution!

Having arbitrary clipping planes sounds like a really nice candy, on top of the already great feature of combining volumetric and polygonal data!

@aarbelaiz Please tell me when this is ready for merge - and thanks a lot for taking the time to check this stuff!

Also, thanks for providing a test case.

As soon as this is working inside the dev branch, it would be really cool to also have an example, which we could put on examples.x3dom.org.

@mlimper

This comment has been minimized.

Contributor

mlimper commented Aug 31, 2015

@aarbelaiz Thanks for the helpful comments and suggestions - please let me know if you think this is ready for merge.

@mlimper

This comment has been minimized.

Contributor

mlimper commented Oct 16, 2015

@aarbelaiz any update? can we merge this one?

@aarbelaiz

This comment has been minimized.

Contributor

aarbelaiz commented Oct 19, 2015

Sorry @mlimper for the delay, in the mailing list http://sourceforge.net/p/x3dom/mailman/message/34428520/ I gave my opinion explaining where this contribution should be headed to. I though @ankit1089 and his co-workers agreed to them, if this is correct I would propose to change the name of the node to RadarVolumeStyle and explicitly state in the documentation that this node is a X3DOM node, and not a X3D node.

In the future, I would like to see the functionalities of this node available in the regular X3D nodes so that a non-standard additional node is required. In the meantime, the proposed node could be merged knowing it could be subject to change in future versions of x3dom.

@mlimper

This comment has been minimized.

Contributor

mlimper commented Oct 19, 2015

Cool, thanks for the feedback.
So we basically just need the node to be renamed and this is ready for merge?

I really like the feature of combining volume renderings and rendering of polygons, this is a very nice feature, and I belive X3DOM users will really like it.

@ankit1089 ankit1089 force-pushed the ankit1089:depth_mapped_volume_style branch from 0da6710 to 6546395 Oct 21, 2015

@ankit1089

This comment has been minimized.

ankit1089 commented Oct 21, 2015

@mlimper @aarbelaiz Sorry for delaying this merge.

As discussed with Ander Arbelaiz in the mailing list, we are trying to implement the combined rendering of polygons and volume data in the "X3DVolumeDataNode" so that other volume styles can also use this functionality. Also, we are trying to implement clip planes for the volume data using the already existing ClipPlane node. We are working on the new code at : https://github.com/ankit1089/x3dom/tree/depthTexture_volumeData but it is still buggy and in development.

Right now both of these functionalities are implemented in the shader of the DepthMappedVolumeStyle (renamed to RadarVolumeStyle) of this pull request.

I renamed the node to "RadarVolumeStyle" for this pull request to get merged. Later on, we would like to replace it with a better implementation.

@mlimper

This comment has been minimized.

Contributor

mlimper commented Oct 21, 2015

As discussed with Ander Arbelaiz in the mailing list, we are trying to implement the combined rendering of polygons and volume data in the "X3DVolumeDataNode" so that other volume styles can also use this functionality

This is a really exciting feature, and I believe a lot of X3DOM users will be very happy to have it. It will also be an interesting key feature of X3DOM. Please keep us updated, as soon as your code becomes stable enough for merge.

I'll now merge this one then. Do you have an example which I could put on examples.x3dom.org to showcase this first version of combined rendering (volumes + polygons)?

mlimper pushed a commit that referenced this pull request Oct 21, 2015

@mlimper mlimper merged commit e5d49a8 into x3dom:master Oct 21, 2015

@ankit1089

This comment has been minimized.

ankit1089 commented Oct 26, 2015

@mlimper Thanks for the merge! I will make a demo for this email it you.

@mlimper

This comment has been minimized.

Contributor

mlimper commented Oct 26, 2015

perfect, looking forward.

@andreasplesch

This comment has been minimized.

Contributor

andreasplesch commented Jan 22, 2016

I am using RadarVolumeStyle to great effect, all works as expected (except or #537). So this is more a request for enhancement or instruction what would could be done to the shader to avoid these kind of artefacts along oblique cross-sections:

vrmath2x4d

These are most obvious for intensity=1 but also show up for smaller intensities. Is there a place in the code to somehow supersample the texture ?

Here is the full scene:
http://googledrive.com/host/0BwIhFzkLaQ9XRmtHbmZQVmFmNHM/CFM5_CVMH3d.html

@aarbelaiz

This comment has been minimized.

Contributor

aarbelaiz commented Jan 25, 2016

Those artifacts are to be expected, you can increase the number of Steps of the ray casting to achieve a better result with more sampling.
Build your x3dom-full.js file, then find & replace Steps = 60.0; with Steps = 120.0;.

@andreasplesch

This comment has been minimized.

Contributor

andreasplesch commented Jan 25, 2016

Ok, changing to Step =120.0; in X3DVolumeData.js works great. I created a simple Greasemonkey user script here:

https://gist.github.com/andreasplesch/4b2742e6260cde371c31

Install by clicking on Raw. This way an unaltered x3dom-full.js can be used.

I think it would be really useful if the Steps parameter could be exposed as a SFFloat field to X3DVolumeData, perhaps called "steps" or "rays" or "quality" . The default could be 60.0. Some sanity checking may be necessary but the assumption should be that a user knows what he is doing if she is changing the default value. Perhaps a log warning if larger than 240 and smaller than 15 ?

vrmath2x4d

@aarbelaiz

This comment has been minimized.

Contributor

aarbelaiz commented Jan 26, 2016

The "steps" | "quality"... field is not in the X3D specification, it is more related to the implementation, thus there is no such argument in the X3DVolume nodes... If added it would be a X3DOM argument only. Do you still see it necessary?

@andreasplesch

This comment has been minimized.

Contributor

andreasplesch commented Jan 26, 2016

Yes, I still think such a field would be very useful. Authors who want a spec. compliant scene do not need to use it. Authors who want to take advantage will know that it is x3dom only. There are many other x3dom only fields and nodes which work by the same principle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment