Skip to content

Conversation

clairekuang
Copy link
Member

@clairekuang clairekuang commented Aug 19, 2024

Because block definition object colors can inherit their colors by layer or by block, and block instance colors can also be by layer or by block, this pr is setting the color for block definition objects as if they are by color in both conditions, to simulate the correct colors in the viewer (and hopefully when receiving in other apps).
NOTE: also sending a source string prop on the color proxies to identify propery color inheritance on receive

includes a refactor for speckle application ids by adding extension methods for layers, entities, materials, colors, etc to guarantee we are always using the same app id.

Original autocad file:
image

Received (old): https://app.speckle.systems/projects/b53a53697a/models/66aca735ab@0836579d97
image

Received (new):
https://app.speckle.systems/projects/b53a53697a/models/66aca735ab@55ecf653c7
image

Copy link

linear bot commented Aug 19, 2024

Copy link

codecov bot commented Aug 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 8.64%. Comparing base (dbc359f) to head (b91eb32).
Report is 1 commits behind head on dev.

Additional details and impacted files
@@          Coverage Diff          @@
##             dev    #167   +/-   ##
=====================================
  Coverage   8.64%   8.64%           
=====================================
  Files        236     236           
  Lines       4490    4490           
  Branches     526     526           
=====================================
  Hits         388     388           
  Misses      4085    4085           
  Partials      17      17           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@didimitrie
Copy link
Member

includes a refactor for speckle application ids by adding extension methods for layers, entities, materials, colors, etc to guarantee we are always using the same app id.

finally! this is super nice to have!

@oguzhankoral
Copy link
Member

I suggest to use another proxy class for ColorSource. By this way, viewer also can know which source it will apply to each object, layer? block? etc.. And I believe this is the clearest approach. Otherwise we're not telling anything to viewer render any geometry from what source. WDYT @didimitrie ?

{
  "colorSourceProxies": [
    {
      "speckle_type": "ColorSourceProxy",
      "name": "ByLayer",
      "objects": ["12345", "12346", "12347", "12348"]
    },
    {
      "speckle_type": "ColorSourceProxy",
      "name": "ByBlock",
      "objects": ["22345", "22346", "22347", "22348"]
    }
  ]
}

@clairekuang
Copy link
Member Author

clairekuang commented Aug 19, 2024

I suggest to use another proxy class for ColorSource. By this way, viewer also can know which source it will apply to each object, layer? block? etc.. And I believe this is the clearest approach. Otherwise we're not telling anything to viewer render any geometry from what source.

Currently I'm attaching the source dynamically to the colorproxy:
image
I think this is fine for now, source can have three values (block, layer, object) and is factored into the color proxy id: currently receives perfectly in autocad and with the change in send logic (including definition object colors when it is by layer), I believe it should also render properly in the current viewer logic

@clairekuang clairekuang requested a review from didimitrie August 19, 2024 16:13
@clairekuang clairekuang merged commit 8a0dfca into dev Aug 20, 2024
5 checks passed
@clairekuang clairekuang deleted the claire/cnx-308-colors-and-render-materials-not-sent-well-in-blocks-from branch August 20, 2024 11:39
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.

3 participants