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: general cylinder inside rendering #2620

Merged

Conversation

haakonflatval-cognite
Copy link
Contributor

Type of change

Bug

Jira ticket 📘

https://cognitedata.atlassian.net/browse/REV-564

Description 📝

Fixes general cylinder when the camera is inside the cylinder. Also fixes the normal direction when looking at the inside of the cylinder.

How has this been tested? 🔍

By looking

Test instructions ℹ️

cd viewer && yarn && cd packages/sector-parser && yarn start

Have a good hard look

Checklist ☑️

  • I am proud of this feature.
  • I have performed a self-review of my own code.
  • I have added PR type (Feat, Bug, Chore, Test, Docs, Style, Refactor) to the PR title.
  • I have manually tested this for different scenarios (different models, formats, devices, browsers).
  • I have commented my code in hard-to-understand areas.
  • I have made corresponding changes to the public documentation.
  • I have added unit and visuals tests to prove that my feature works to the best of my ability.
  • I have refactored the code for readability to the best of my ability.
  • I have checked that my changes do not introduce regressions in the public documentation.
  • I have outlined any known defects / lacking capabilities in the contents of this PR.
  • I have listed any remaining work that should follow this PR in the description or jira/miro/etc.
  • I have added TSDoc to any public facing changes.
  • I have added "developer documentation" in any package README.md that is related / applicable to this PR.
  • I have noted down and am currently tracking any technical debt introduced in this PR.
  • I have thoroughly thought about the architecture of this implementation.
  • I have asked peers to test this feature.

@haakonflatval-cognite haakonflatval-cognite added the slack PRs and issues with this label will be pushed to Slack label Oct 28, 2022
@haakonflatval-cognite haakonflatval-cognite requested a review from a team as a code owner October 28, 2022 11:16
@github-actions
Copy link

There were failures in the visual tests workflow.
Image diffs for visual tests can be downloaded as an artifact here.

@github-actions
Copy link

There were failures in the visual tests workflow.
Image diffs for visual tests can be downloaded as an artifact here.

@codecov
Copy link

codecov bot commented Oct 28, 2022

Codecov Report

Merging #2620 (f4d63c0) into master (386371f) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2620   +/-   ##
=======================================
  Coverage   69.56%   69.57%           
=======================================
  Files         254      254           
  Lines       10348    10348           
  Branches     1331     1331           
=======================================
+ Hits         7199     7200    +1     
+ Misses       2980     2979    -1     
  Partials      169      169           
Impacted Files Coverage Δ
...r/packages/utilities/src/indexset/LeafIndexNode.ts 84.44% <0.00%> (+2.22%) ⬆️

@haakonflatval-cognite haakonflatval-cognite merged commit 59a6f17 into master Oct 31, 2022
@haakonflatval-cognite haakonflatval-cognite deleted the hflatval/general-cylinder-inside-rendering branch October 31, 2022 09:24
christjt pushed a commit that referenced this pull request Nov 2, 2022
* fix: rendering if camera is inside primitive

* fix: reverse normal direction if hitting inside of cylinder
christjt added a commit that referenced this pull request Nov 3, 2022
…ch (#2635)

* chore: add workflow that deploys to NPM on Github release (#2586)

* test: initial dry run publish

* chore: trigger release on released published

* fix: remove caching on release workflow

Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com>

* fix: expose and fix bug causing getBoundingBoxByNodeId/TreeIndex not to modify out parameter (#2472)

* refact: keyboard and mouse event handling in viewer (#2492)

* Intermediate commit

* Removed isFocused check in keyboard handling

* Keyboard active only which domElement is focused, mouse events are active when canvas is focused

* Refactored axisview eventlistener

* Reverted axisview event passed to domElement

* updated examples yarn.lock

* clean up

* leftover clean up

* Keyboard event listen on domElement, removed unnecessary event push to domElement from Axis cross

* removed unused variables

* clean up

* Fixed visual test error

Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com>

* fixed point cloud picking precision issue (#2508)

Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com>

* improvement: use 'high-performance' powerPreference to avoid non-discrete GPU being used on certain systems (#2512)

* feat: add metrics to MeasurementTool (#2523)

* chore: backport to old file structure

* fix: remove flat modifier, while maintaining TreeIndex precision (#2536)

Contribution by @Strepto (Equinor)

* fix: do not blend in-front and back frame buffers when back objects have not been rendered (#2540)

Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com>

* fix: visualization artifacts due to wrongfully handling cylinder clip planes (#2556)

* fix: simplify shader

* fix: set proper plane length

* fix: reduce some matrix transformations, and use built-in face-forward

* fix: plane magnitude not adjusting for scaled world transform

* fix: bad merge

* chore: revert CDF environments json

* chore: add primitive test fixture

Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com>

* improvement: make CadModelUpdateHandler trigger sector loading immediately after the camera stops moving (#2573)

* Make CadModelUpdateHandler trigger sector loading immediately after the camera stops moving

* Remove setting of redraw flag

* Update test

* Fix test

Co-authored-by: Christopher J. Tannum <[email protected]>

* Use vec2 instead of struct for packing TreeIndexes (#2585)

This works around an issue with struct precision on ardeno gpus: KhronosGroup/WebGL#3351

* fix: general cylinder inside rendering (#2620)

* fix: rendering if camera is inside primitive

* fix: reverse normal direction if hitting inside of cylinder

* fix: enable depth writing for in-front pass (#2621)

* Enable depth writing for in-front pass

* Set depth to minimum of in-front and back pass

Co-authored-by: Christopher J. Tannum <[email protected]>

* fix: remove unused pcMaterialManager from bad merge

* fix: re-create v8 test image due to cylinder clipping planes fix

Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: Lars Moastuen <[email protected]>
Co-authored-by: Pramod S <[email protected]>
Co-authored-by: Nils Henrik Hals <[email protected]>
Co-authored-by: eiriklegernaes <[email protected]>
Co-authored-by: Håkon Flatval <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
slack PRs and issues with this label will be pushed to Slack
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants