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

fix grid-absolute for off-center models #1049

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

snoyer
Copy link
Contributor

@snoyer snoyer commented Oct 11, 2023

absolute-grid option currently works as intended when models are off-center vertically (in the up direction) but not when they are off-center "sideways" (along the grid plane), see example below:

absolute-grid-bug

This PR changes the behavior to the following:

absolute-grid-fix

axes are still in the same position at the origin, but the actor and fading radius of the grid are centered below the model.

There is still room for improvement, for example if the model is very far from the origin the axes may not be visible at all (outside of radius), solving that would require more involved framing/fading patterns of the grid plane.

This fix should at least make the feature not broken anymore.

@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Merging #1049 (4fe3f9b) into master (07d6c6e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1049   +/-   ##
=======================================
  Coverage   96.22%   96.22%           
=======================================
  Files         120      120           
  Lines        7015     7023    +8     
=======================================
+ Hits         6750     6758    +8     
  Misses        265      265           
Files Coverage Δ
...VTKExtensions/Rendering/vtkF3DOpenGLGridMapper.cxx 99.00% <100.00%> (+0.03%) ⬆️
...y/VTKExtensions/Rendering/vtkF3DOpenGLGridMapper.h 100.00% <100.00%> (ø)
library/VTKExtensions/Rendering/vtkF3DRenderer.cxx 99.86% <100.00%> (+<0.01%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@Meakk Meakk left a comment

Choose a reason for hiding this comment

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

Indeed, fading based on the model bbox looks better than fading based on the grid origin.

@Meakk Meakk merged commit f007bff into f3d-app:master Oct 11, 2023
41 checks passed
@snoyer snoyer deleted the fix-grid-absolute branch October 11, 2023 15:19
mwestphal pushed a commit that referenced this pull request Feb 10, 2024
mwestphalnew pushed a commit to mwestphalnew/f3d that referenced this pull request Feb 10, 2024
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.

2 participants