-
-
Notifications
You must be signed in to change notification settings - Fork 399
Add control for coordinate precision shifting and scaling and fix widgets #3345
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
base: master
Are you sure you want to change the base?
Conversation
7e85980 to
d087ea6
Compare
…aling Introduces the `(set|get)CoordinateShiftScaleEnabled` property to `vtkMapper` and its subclasses. This new property provides an API to explicitly enable/disable the automatic coordinate shifting and scaling mechanism within polydata mappers, which is typically used to improve rendering precision for geometry with large coordinate values. fix Kitware#3330
The glyph representation is primarily used for widget assembly where there the infinite lines or smaller diagonal length would trigger an unexpected coordinate shift scale. This change disables this by default. fix Kitware#3330
d087ea6 to
488f970
Compare
|
|
||
| // Build VTK API | ||
| macro.setGet(publicAPI, model, ['context']); | ||
| macro.setGet(publicAPI, model, ['context', 'coordShiftScaleEnabled']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
model.coordShiftScaleEnabled is not initialized on purpose ?
Therefore false by default ?
Why would coordShiftScaleEnabled have a different value between the API specific mapper (vtkOpenGLPolyDataMapper) and the original mapper (vtkPolyDataMapper)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be added in the documentation.
|
|
||
| /** | ||
| * Control whether the mapper should automatically shift and scale | ||
| * the coordinates to improve precision. This is enabled by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add comment to explain when/why you would want to turn it off.
| let coordShift = 0.0; | ||
| let coordScale = 1.0; | ||
|
|
||
| if (model.coordShiftAndScaleEnabled && pts) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why don't you check the value of the original mapper (vtkPolyDataMapper.coordShiftAndScaleEnabled), but just the value of the API specific value (vtkOpenGLPolyDataMapper.coordShiftAndScaleEnabled)
| if (model.coordShiftAndScaleEnabled && pts) { | ||
| ({ useShiftAndScale, coordShift, coordScale } = | ||
| computeCoordShiftAndScale(pts)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of having a flag, could you detect when shift/scale have "bad" values ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe a metric could be computed with regard to the current camera settings.
An error in the "display world" could be considered to too large and coord shift/scale be therefore discarded ?
This `coordShiftScaleEnabled` property is now accessed via `model.renderable.getCoordShiftScaleEnabled()` in `Glyph3DMapper`.
Context
See #3330
Results
Changes
PR and Code Checklist
npm run reformatto have correctly formatted codeTesting