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

Feature/hatching #1604

Closed
wants to merge 11 commits into from
Closed

Feature/hatching #1604

wants to merge 11 commits into from

Conversation

ijlal99
Copy link
Collaborator

@ijlal99 ijlal99 commented Jul 31, 2024

  • Users can now instantiate SectionCapsPlugin along with their section planes to enable hatching on the scene
  • This plugin supports multiple section planes and models on the scene

@ijlal99 ijlal99 requested a review from xeolabs July 31, 2024 20:30
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>xeokit Example</title>
<link href="../css/pageStyle.css" rel="stylesheet" />
<script src="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/5.13.0/js/all.min.js"></script>

Check warning

Code scanning / CodeQL

Inclusion of functionality from an untrusted source Medium

Script loaded from content delivery network with no integrity check.
Copy link
Member

@xeolabs xeolabs left a comment

Choose a reason for hiding this comment

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

Thanks Muhammad! Only changes I'd like to make is to make all methods and variables that are used internally by the plugin private, and remove them from the TS definitions. Then it would seem to have a nice simple API, as an automatic background helper. Also, please add a method, like setEnabled(), so the user can disable/enable the plugin dynamically.

src/plugins/SectionCapsPlugin/SectionCapsPlugin.js Outdated Show resolved Hide resolved
src/plugins/SectionCapsPlugin/SectionCapsPlugin.js Outdated Show resolved Hide resolved
src/plugins/SectionCapsPlugin/SectionCapsPlugin.js Outdated Show resolved Hide resolved
src/plugins/SectionCapsPlugin/SectionCapsPlugin.js Outdated Show resolved Hide resolved
src/viewer/Viewer.js Outdated Show resolved Hide resolved
@xeolabs
Copy link
Member

xeolabs commented Aug 7, 2024

Got a strange little artifact when testing examples/slicing/SectionPlanesPlugin_Duplex_SectionCaps.html

Screenshot from 2024-08-07 17-08-22
Screenshot from 2024-08-07 17-08-03

@ijlal99
Copy link
Collaborator Author

ijlal99 commented Aug 7, 2024

@xeolabs this is something that is caused by objects that are not solids but classified as solids (I believe).

@xeolabs
Copy link
Member

xeolabs commented Aug 7, 2024

@xeolabs this is something that is caused by objects that are not solids but classified as solids (I believe).

Gotcha - can you make it not make solid caps for non-solid objects?

@ijlal99
Copy link
Collaborator Author

ijlal99 commented Aug 7, 2024

@xeolabs that's how I am currently doing it. I am only generating caps for objects that are classified as solids.

const isSolid = sceneModel.objects[key].meshes[0].layer.solid ?? false;

@xeolabs
Copy link
Member

xeolabs commented Aug 7, 2024

@xeolabs that's how I am currently doing it. I am only generating caps for objects that are classified as solids.

const isSolid = sceneModel.objects[key].meshes[0].layer.solid ?? false;

Aha I see. These objects are wrongly classified.

@ijlal99
Copy link
Collaborator Author

ijlal99 commented Aug 7, 2024

I am not sure if I can do much about it but would love to hear your ideas.

@xeolabs
Copy link
Member

xeolabs commented Aug 7, 2024

Here's an XKT with properly classified solids:

model.zip

However, it would also be good to have an example that demonstrated solid caps with a programmatically-created SceneModel, like this example: https://xeokit.github.io/xeokit-sdk/examples/scenemodel/#vbo_instancing_autocompressed_triangles

The idea is that we always have a minimal example that's easier to debug (way easier than with an IFC model).

Note that you'd need to change the primitive types on the SceneModel meshes in that example to indicate that they are solids.

@ijlal99
Copy link
Collaborator Author

ijlal99 commented Aug 19, 2024

  1. I fixed the issue with hatching that was causing some random artifacts to show alongside the hatched model.
  2. I have added another parameter with 'opacityThreshold' for user to control hatching the objects that have opacity above this threshold.
  3. There was a bug in the implementation of getEachVertex that was not correctly transforming the vertices.
  4. I have added a simple example as suggest by @xeolabs but the example with skt model is also there. If you guys want, I can delete it.

Attached is the result of how hatching looks on previous xkt model, the model provided by @xeolabs and a more simpler model.

Screenshot 2024-08-19 at 10 01 26 PM Screenshot 2024-08-19 at 10 02 19 PM Screenshot 2024-08-19 at 10 04 01 PM

examples/slicing/SectionCapsPlugin.html Dismissed Show dismissed Hide dismissed
@MichalDybizbanskiCreoox
Copy link
Collaborator

I'd suggest reorganizing commits a bit, to avoid introducing unstable state of the repository.
An example would be
8acb036#diff-3986a1ed352df2aeac30a1e1db927c1a2285e4ce219ec61dfa57406f01ef4983
which introduces a import html2canvas from '../../node_modules/html2canvas/dist/html2canvas.esm.js'; regression, that is rectified by
e143581
The latter could be merged with the former to prevent introducing a temporary regression to the master branch.

@Amoki
Copy link
Contributor

Amoki commented Aug 23, 2024

Could it be possible to color or texturize the hatch depending on the IfcType of the object?

@ijlal99 ijlal99 marked this pull request as draft August 25, 2024 07:11
@ijlal99 ijlal99 closed this Aug 27, 2024
@ijlal99 ijlal99 deleted the feature/hatching branch August 27, 2024 05:55
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.

5 participants