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.

Sign up for GitHub

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

Jump to bottom

[Pointcloud] Fixes in Pointcloud Gem #62

Merged
merged 6 commits into from
Sep 18, 2024
Merged

[Pointcloud] Fixes in Pointcloud Gem #62

merged 6 commits into from
Sep 18, 2024

Conversation

zakmat
Copy link
Member

@zakmat zakmat commented Sep 5, 2024

This PR applies several fixes into Pointcloud Gem:

  • rectifies a regression where PointcloudConfigurationBus was not enabled for Editor Component
  • Removes log flooding with failed re-enable of feature processor by querying before enable attempt
  • fixes bounds calculation for invalid pointclouds - applying transform was making them valid but incorrect.

The PR also cleans up headers a little and add some comments

@zakmat
enable pointcloud bus for editor component
a9c15ab 
Signed-off-by: Mateusz Żak <[email protected]>
@zakmat
clean up
5338e09 
* add comments
* clean up headers

Signed-off-by: Mateusz Żak <[email protected]>
@zakmat
query for featureProcessor before enable attempt
96c6232 
* this avoids flooding logs with warnings

Signed-off-by: Mateusz Żak <[email protected]>
@zakmat
do not apply transforms on invalid bounds
856d22a 
* it will make them valid - but incorrect

Signed-off-by: Mateusz Żak <[email protected]>
@zakmat
further cleanup
86b6162 
* shorten the Gems Bus header filename

Signed-off-by: Mateusz Żak <[email protected]>
norbertprokopiuk
norbertprokopiuk reviewed Sep 16, 2024
View reviewed changes
Copy link
Contributor

@norbertprokopiuk norbertprokopiuk left a comment

Choose a reason for hiding this comment

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

Most of included changes are fine but i have some doubts about local bounds checking. current solution makes it impossible to select point cloud if point (0,0,0) is outside of camera frustum

Gems/Pointcloud/Code/Source/Tools/Components/PointcloudEditorComponent.cpp Show resolved Hide resolved
@zakmat
Add ChangeEvent to PointcloudFeatureProcessor
76d856a 
* asset change should be signaled to the PointcloudController
* currently the change is needed to update cached local bounds to fix selection

Signed-off-by: Mateusz Żak <[email protected]>
@zakmat zakmat force-pushed the mzak/fix_pointcloud_editor_bus branch from cdb44a3 to 76d856a Compare September 17, 2024 09:12
norbertprokopiuk
norbertprokopiuk approved these changes Sep 18, 2024
View reviewed changes
Copy link
Contributor

@norbertprokopiuk norbertprokopiuk 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 and works as expected

@zakmat zakmat merged commit 6d6a10b into main Sep 18, 2024
1 check passed
@zakmat zakmat deleted the mzak/fix_pointcloud_editor_bus branch September 18, 2024 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@norbertprokopiuk norbertprokopiuk norbertprokopiuk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zakmat @norbertprokopiuk