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

Matcap #23

Merged
merged 23 commits into from Jan 27, 2020
Merged

Matcap #23

merged 23 commits into from Jan 27, 2020

Conversation

mbotsch
Copy link
Member

@mbotsch mbotsch commented Jan 11, 2020

The PR adds mat-cap textures to src/pmp/visualization/SurfaceMeshGL. Since they are a special kind of spherical environment map, the are used in the rendering mode "Texture".

The external repository pmp-data has been updated to include some of Blender's mat-cap files. Find them in external/pmp-data/matcap/.

Currently mview is the only application using mat-caps, by specifying -m matcap-texture.jpg as command line parameter. We maybe should add a common command line parsing to all applications that derive from MeshViewer...

The PR also improves the parameter passing and file loding in case of an emscripten WebAssembly build. The mview web-application now supports to add these parameters in the URL's query part:

  • model=my_model.off
  • texture=my_texture.jpg
  • material=my-matcap.jpg

When the web-app is embedded using an iframe, these attributes can also be added the element attributes to the iframe.

@mbotsch mbotsch requested a review from dsieger January 11, 2020 14:25
@coveralls
Copy link

coveralls commented Jan 11, 2020

Coverage Status

Coverage remained the same at 91.352% when pulling 1788e14 on matcap into 09532b4 on develop.

Copy link
Member

@dsieger dsieger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, only minor comments

@@ -0,0 +1,63 @@
//=============================================================================
// Copyright (C) 2011-2018 The pmp-library developers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copyright should be bumped to 2020

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for new files or for changed files?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be updated to current year on every change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated for all files that changed

" vec3 N = normalize(v2f_normal);\n"
" vec3 V = normalize(v2f_view);\n"
" \n"
//" if (!gl_FrontFacing) N = -N;\n" // gl_FrontFacing does not work on Mario's new MacBook
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code should be removed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code will be removed anyway. the new one does not have dead lines.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, my bad.

@@ -40,7 +41,7 @@ SurfaceMeshGL::SurfaceMeshGL()

// material parameters
front_color_ = vec3(0.6, 0.6, 0.6);
back_color_ = vec3(0.5, 0.0, 0.0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? The red rendering of back faces is quite useful, e.g., for detecting intersections or fold-overs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't know anymore. something didn't work. let's change it back.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I just pushed a change for that.

@dsieger dsieger merged commit 35988ab into develop Jan 27, 2020
@dsieger dsieger deleted the matcap branch January 27, 2020 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants