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

Generic Rendering #101

Merged
merged 11 commits into from
Nov 7, 2024
Merged

Generic Rendering #101

merged 11 commits into from
Nov 7, 2024

Conversation

jmwright
Copy link
Contributor

@jmwright jmwright commented Nov 5, 2024

@julianstirling The build/compile CI Action is giving an error that suggests that I've broken off-screen rendering somehow, and I'm not sure how I did it. I have also somehow broken glb/glTF export, most likely because of the changes I made to generate_assembly_model. I'm not super excited about that change (passing rendering_options around) because it blurs some concepts together, but it was the most direct method I could think of to facilitate the generic rendering mechanism. I'm out of time to work on this right now, and may not be able to get these things fixed before tomorrow.

Screenshot from 2024-11-05 16-10-26

Closes #100

@julianstirling
Copy link
Collaborator

Do you want me to look at getting the pipeline to run tomorrow?

@jmwright
Copy link
Contributor Author

jmwright commented Nov 6, 2024

Do you want me to look at getting the pipeline to run tomorrow?

I finally got it, thanks. I couldn't get xvfb-run to accept the json, but I finally figured out the incantation of escaped quotes to get it to pass through. Going to need a squash on this merge for sure.

@julianstirling
Copy link
Collaborator

If you don't want to squash the whole thing you can do git rebase --interactive master To rebase on the same master it is currently based on. This will bring up a screen where you can select past commits to skip.

@jmwright
Copy link
Contributor Author

jmwright commented Nov 6, 2024

@julianstirling I have fixed the bugs that I introduced, but I still have two issues (the second one is minor).

  1. I am trying to create a renders directory within assembly-docs to put the png renders. I can see it pop up in the directory listing for an instant, and then it seems to get purged during some sort of cleanup when I run cadorchestrator generate .... I'm assuming this directory needs to be created via a configuration file rather than directly in code, but I'm not sure where to do it.
  2. When I run cadorchestrator serve, the browser (Firefox) seems to be caching an old assembly.glb file, even though I've deleted the _cache_ and build directories. I opened the link in a different browser (Chrome) that hadn't seen the file yet, and it came up correctly. I also dragged the assembly.glb file to an online viewer, and it looks correct. As a side note, the online viewer seems to have the correct orientation for the model. The problem with orientation in the viewer is probably a hold-over from when I originally prototyped the UI, unless you redid that code.

@julianstirling
Copy link
Collaborator

@jmwright Is this ready for review?

@jmwright
Copy link
Contributor Author

jmwright commented Nov 7, 2024

@julianstirling Yes, it's ready.

Copy link
Collaborator

@julianstirling julianstirling left a comment

Choose a reason for hiding this comment

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

One suggestion as GitBuilding eats the renders after they are generated. Suggested code tested locally here.

A few comments, but they don't need to delay merging.

mechanical/assembly_renderer.py Outdated Show resolved Hide resolved
nimble_build_system/cad/renderer.py Outdated Show resolved Hide resolved
nimble_build_system/cad/renderer.py Outdated Show resolved Hide resolved
nimble_build_system/cad/shelf.py Show resolved Hide resolved
tests/test_rendering.py Show resolved Hide resolved
@jmwright jmwright merged commit c334972 into master Nov 7, 2024
5 checks passed
@jmwright jmwright deleted the generic_rendering branch November 7, 2024 17:23
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.

Render refactor
2 participants