h3n3/cadhub
1
0
Fork 0
Code Issues 35 Pull Requests Packages Projects Releases Wiki Activity

Adjust openscad image to match viewer size #239

New Issue
Closed
opened 2021-03-12 20:33:31 +01:00 by Irev-Dev · 11 comments
Irev-Dev commented 2021-03-12 20:33:31 +01:00 (Migrated from github.com)
Copy Link

web/src/helpers/cadPackages/openScadController.js is hardcoded to get a 500x500 imaged from the openscad renderer. This should instead match the size of the viewer panel. I would assume that react-mosaic has some kind of event for resizes that we can fire to adjust it.

It should also detect if the device is retina display and double the resolution.
https://stackoverflow.com/questions/19689715/what-is-the-best-way-to-detect-retina-support-on-a-device-using-javascript#20413768

`web/src/helpers/cadPackages/openScadController.js` is hardcoded to get a 500x500 imaged from the openscad renderer. This should instead match the size of the viewer panel. I would assume that `react-mosaic` has some kind of event for resizes that we can fire to adjust it. It should also detect if the device is retina display and double the resolution. https://stackoverflow.com/questions/19689715/what-is-the-best-way-to-detect-retina-support-on-a-device-using-javascript#20413768
franknoirot commented 2021-03-14 02:40:47 +01:00 (Migrated from github.com)
Copy Link

Hey @Irev-Dev in openScadController.js, is the size property supposed to control the size of the image returned from OpenSCAD? I've been able to get the image full-size and added in the viewerSize based on the on-screen size of the element to IdeState, passing it into the render call, but I'm still getting a 512 x 512px image back from OpenSCAD. I'm assigning the x and y values on lines 23 and 24 of openScadController.

Hey @Irev-Dev in `openScadController.js`, is the `size` property supposed to control the size of the image returned from OpenSCAD? I've been able to get the image full-size and added in the `viewerSize` based on the on-screen size of the element to `IdeState`, passing it into the render call, but I'm still getting a 512 x 512px image back from OpenSCAD. I'm assigning the `x` and `y` values on lines 23 and 24 of `openScadController`.
Irev-Dev commented 2021-03-14 06:22:10 +01:00 (Migrated from github.com)
Copy Link

I'm guessing 512 is the default for openscad, I've had a quick look, and the number look like floating points to me not integers. so I'm guessing it's falling back to 512. Try rounding the numbers to integers.

I'll back to this in a couple hours. I should be able to tell you something more definite. ^ that's my suspicion.

I'm guessing 512 is the default for openscad, I've had a quick look, and the number look like floating points to me not integers. so I'm guessing it's falling back to 512. Try rounding the numbers to integers. I'll back to this in a couple hours. I should be able to tell you something more definite. ^ that's my suspicion.
👍 1
franknoirot commented 2021-03-14 07:22:56 +01:00 (Migrated from github.com)
Copy Link

Gold, that did the trick. I'll push up a commit now and then get the retina thing sorted in a final one before PR.

Gold, that did the trick. I'll push up a commit now and then get the retina thing sorted in a final one before PR.
franknoirot commented 2021-03-14 07:51:45 +01:00 (Migrated from github.com)
Copy Link

@Irev-Dev okay so now I'm on another issue, seems related to a comment you left on line 118 of IdeViewer.js which I've tried to demonstrate in my latest commit. When the panes are dragged the image now corrects by fetching a new image ✔, but after having dragged the pane if you try to orbit the camera the preview uses a stale viewerSize value and goes back to incorrect proportions. You mentioned in that comment the need to use a locally-defined variable to overcome the stale code issue within this file, but though I've been able to do the same with viewerSize within the component itself, what is interesting to me is that the render dispatched event within ideViewer uses a stale viewerSize value even when that value is not passed into the render call, but used within useIde. I'll investigate more in the morning just wanted you to see what I'm working through, because it might be something a bit larger to sort out now before this stale state thing gives us perennial headaches.

@Irev-Dev okay so now I'm on another issue, seems related to a comment you left on line 118 of `IdeViewer.js` which I've tried to demonstrate in my latest commit. When the panes are dragged the image now corrects by fetching a new image ✔, but after having dragged the pane if you try to orbit the camera the preview uses a stale `viewerSize` value and goes back to incorrect proportions. You mentioned in that comment the need to use a locally-defined variable to overcome the stale code issue within this file, but though I've been able to do the same with `viewerSize` within the component itself, what is interesting to me is that the `render` dispatched event within `ideViewer` uses a stale `viewerSize` value even when that value is not passed into the `render` call, but used within `useIde`. I'll investigate more in the morning just wanted you to see what I'm working through, because it might be something a bit larger to sort out now before this stale state thing gives us perennial headaches.
👀 1
Irev-Dev commented 2021-03-14 08:00:06 +01:00 (Migrated from github.com)
Copy Link

@franknoirot I had a bit of a play earlier and got something working.

One . . . interesting thing I did is to manage the viewer-dimension-state completely in the openscad controller so that openscad specific state isn't dirtying the ide state. It makes other things more complicated. e.g. When initing, the controller is expecting both the ideContainer and the viewer to call render, one gives it it's dimensions and the other it's camera and it doesn't return an image until the second render call, but other than that it also means it automatically renders a new image on resize.

So I think I've likely side stepped the issue you've run into. but I had ran into stale state in ideViewer too (I think somehow react-three-fiber not being completely react through-and-through might be causing it) And 100% you found my work around on 118, raising a variable to module scope. I noticed that If I logged out the state inside of IdeViewer it was showing the correct state, but when the callback onCameraChange fired it would be state, so but updating a module level variable. Even though onCameraChange is my own callback, it original spawns from line 77 controls.current.addEventListener('end', dragCallback) and I guess this is from with three.js and yeah not sure why.

Your point about it using the stale value even when not passed viewerSize. I think I ran into that too at some point. It's really confusing, not sure I can be of much help

If my work is of use to you you can checkout kurt/239 or look at the commit

Demo of rotate -> resize -> rotate working

https://user-images.githubusercontent.com/29681384/111061307-c06b2280-84f6-11eb-8699-1a0af5c745dc.mov

Does that help?

@franknoirot I had a bit of a play earlier and got something working. One . . . interesting thing I did is to manage the viewer-dimension-state completely in the openscad controller so that openscad specific state isn't dirtying the ide state. It makes other things more complicated. e.g. When initing, the controller is expecting both the ideContainer and the viewer to call render, one gives it it's dimensions and the other it's camera and it doesn't return an image until the second render call, but other than that it also means it automatically renders a new image on resize. So I think I've likely side stepped the issue you've run into. but I had ran into stale state in ideViewer too (I think somehow `react-three-fiber` not being completely react through-and-through might be causing it) And 100% you found my work around on 118, raising a variable to module scope. I noticed that If I logged out the state inside of `IdeViewer` it was showing the correct state, but when the callback `onCameraChange` fired it would be state, so but updating a module level variable. Even though `onCameraChange` is my own callback, it original spawns from line 77 `controls.current.addEventListener('end', dragCallback)` and I guess this is from with three.js and yeah not sure why. Your point about it using the stale value even when not passed viewerSize. I think I ran into that too at some point. It's really confusing, not sure I can be of much help If my work is of use to you you can checkout `kurt/239` or look at the [commit](https://github.com/Irev-Dev/cadhub/commit/b6867fc8a81f01878c1cd0fd0cfde9aa6e41db36?branch=b6867fc8a81f01878c1cd0fd0cfde9aa6e41db36&diff=split) Demo of rotate -> resize -> rotate working https://user-images.githubusercontent.com/29681384/111061307-c06b2280-84f6-11eb-8699-1a0af5c745dc.mov Does that help?
Irev-Dev commented 2021-03-14 08:13:14 +01:00 (Migrated from github.com)
Copy Link

I update the above comment a couple times after re reading your comment.

I update the above comment a couple times after re reading your comment.
franknoirot commented 2021-03-15 06:07:45 +01:00 (Migrated from github.com)
Copy Link

Ah nice @Irev-Dev , I thought the viewerSize would be useful across CAD packages initially but I like what you did way better, that's working great. I think you got it if you want to PR, sorry I couldn't be of more use on this one. Love the new sprinkles lol

Ah nice @Irev-Dev , I thought the `viewerSize` would be useful across CAD packages initially but I like what you did way better, that's working great. I think you got it if you want to PR, sorry I couldn't be of more use on this one. Love the new sprinkles lol
Irev-Dev commented 2021-03-15 07:43:13 +01:00 (Migrated from github.com)
Copy Link

okay, I've put up #251, sorry didn't mean to steal your thunder.

okay, I've put up #251, sorry didn't mean to steal your thunder.
🚀 1
franknoirot commented 2021-03-17 04:59:34 +01:00 (Migrated from github.com)
Copy Link

No no worries!

No no worries!
franknoirot commented 2021-03-17 05:27:41 +01:00 (Migrated from github.com)
Copy Link

@Irev-Dev okay I'm messaging in here before a PR because it seems to simple to be correct. I know you linked to that StackOverflow discussion but what I've put in my commit is inspired by this MDN article about using the Window.devicePixelRatio property to correct the resolution of a canvas, which I all I believe we need correct? Just generate an OpenSCAD render that is the size of the container multiplied by the pixel density? It seems to work on my laptop, which has a pixel ratio of 1.25. Let me know if I'm misunderstanding the problem.

@Irev-Dev okay I'm messaging in here before a PR because it seems to simple to be correct. I know you linked to that StackOverflow discussion but what I've put in [my commit](https://github.com/Irev-Dev/cadhub/tree/franknoirot/239) is inspired by [this MDN article](https://developer.mozilla.org/en-US/docs/Web/API/Window/devicePixelRatio) about using the `Window.devicePixelRatio` property to correct the resolution of a canvas, which I all I believe we need correct? Just generate an OpenSCAD render that is the size of the container multiplied by the pixel density? It seems to work on my laptop, which has a pixel ratio of 1.25. Let me know if I'm misunderstanding the problem.
🚀 1
Irev-Dev commented 2021-03-17 05:32:34 +01:00 (Migrated from github.com)
Copy Link

@franknoirot Sounds like you understand the problem better than me 👏

@franknoirot Sounds like you understand the problem better than me 👏
🎉 1
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
h3n3
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: h3n3/cadhub#239
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?