bartmcleod / VrmlParser

A VrmlParser based on a PEG.js grammar
25 stars 10 forks source link

Some geometries seems to be incomplete - few triangles missing #5

Closed ram-sellamuthu closed 5 years ago

ram-sellamuthu commented 5 years ago

Hi, I was testing the same WRL file with Three.js and VrmlParser.js. I get slightly different visualizations. The WRL file has only one shape primarily IndexedFaceSet. I am herewith attaching the WRL file and also screenshots from two different browsers.

Requesting your inputs in this regard.

01_caster_assembly.zip

rendering_with_threejs rendering_with_vrmlparser

Thanks, Rajaram

ram-sellamuthu commented 5 years ago

@bartmcleod tagging you as I am not sure if you have received any notification from Git. I have imported the same WRL to blender and it looks fine. While rendering using VrmlParser, it looks like some triangles are missing. Looking forward to your response.

bartmcleod commented 5 years ago

@ram-sellamuthu Thanks for tagging me and thanks for your feedback, it is much appreciated. I will look into it. This might either be a serious issue, or just a few flipped triangles (triangles that are specified ccw while all others are cw, or the other way round).

Please note that the VrmlParser library and threejs are not in sync, as my pull requests towards threejs are not merged yet (as far as I know). Keeping them in sync requires a separate pull request. You could verify the outcome based on my branch of threejs, if you like.

bartmcleod commented 5 years ago

@ram-sellamuthu Would it be possible for you to create a test case example that includes just one of the cylinders and presents the same behaviour? [Edit: nvm I can easily isolate a single indexed faceset from the file] [Edit] It is a bug and is has existed for a few years, since the introduction of the new parser. My first guess is that it has to do with the face indexes being specified three on a line, while I tested with only one face specification per line. This might or might not provide a workaround for you, I will test this to see if that is the cause.

bartmcleod commented 5 years ago

@ram-sellamuthu Ok, so here it is: the last face defined in each IndexedFaceSet should also end with -1, (at least for my parser). It might not by strictly needed for the VRML 97 spec.

[Edit]The last set of indices does not need to be followed by , -1 according to the VRML 97 spec, so this is still a bug.

bartmcleod commented 5 years ago

Fixed in v0.7.4

ram-sellamuthu commented 5 years ago

Hi Bart,

Thanks a lot for your quick support and apologies for delayed response. I was in a workshop since the day I raised this issue.

The newer version of VrmlParser works perfectly fine as expected.

Thanks, Rajaram

On Thu, Sep 6, 2018 at 1:55 AM Bart McLeod notifications@github.com wrote:

Closed #5 https://github.com/bartmcleod/VrmlParser/issues/5.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bartmcleod/VrmlParser/issues/5#event-1828834926, or mute the thread https://github.com/notifications/unsubscribe-auth/ApAEnMWVCWsRHhqLLU3Jt_tLg9Ixsl10ks5uYDNGgaJpZM4WaSdm .

-- Thanks & Regards, Rajaram Sellamuthu Cell Phone: +91-91485-48808