bernhard-42 / vscode-ocp-cad-viewer

A viewer for OCP based Code-CAD (CadQuery, build123d) integrated into VS Code
Apache License 2.0
96 stars 10 forks source link

Allow override of FIRST_CALL reset_camera in show_all(...) #29

Closed WhichQuord closed 10 months ago

WhichQuord commented 1 year ago

I suggest a fix/enhancement to allow override of the "reset_camera" option on FIRST_CALL that is happening in show_all(...)

I often run/debug/re-run via the "Run Python FIle" function (which I mapped to F5 key to be closely aligned with OpenSCAD workflow) rather than through a debug interface. But this has the result of always resetting the camera on each run. To mitigate this, I've changed my local copy of ocp-cad-viewer this way to disregard the FIRST_CALL setting if any "reset_camera" option is provided by the caller:

    if FIRST_CALL:
        if kwargs.get("reset_camera") is None:
            kwargs["reset_camera"] = Camera.RESET

and then in my b123d script, I call show_all(reset_camera=Camera.KEEP) which solves the problem for me and keeps the view consistent between runs. And it doesn't seem to interfere with any of the provisions in place for debugging. Perhaps you could add this fix or something similar to override the FIRST_CALL behavior if desired by the caller.

As an added feature, if calling this way, it would be nice if the "Reset View" button in the viewer would behave as an on-demand reset_camera.RESET action and reset the view based on the bounding box. Currently if/when I actually do want a camera reset, I have to add a parameter-less show_all() and re-run to achieve it.

bernhard-42 commented 1 year ago

disregard the FIRST_CALL setting if any "reset_camera" option is provided

I'll have a look. I added this behaviour, since when showing different objects in different runs and their bounding box is very different, strange things could happen. So a proper reset before each run seems appropriate.

How about using the Jupyter approach with separating code parts by # %% and then use Ctrl-Run or Shift-Run again and again on the same same piece of code? It looks like you are trying to simulate that.

the "Reset View" button in the viewer would behave as an on-demand reset_camera.RESET action

This was a design decision that the reset button gets you back to your initial camera position in case you wanted to check something under a different angle. I use this very often and don't want to change this behaviour.

I would really recommend to look at the interactive way of working. Just add # %% at the top of your file and then press Shift-Enter. Another advantage is that it only loads build123d and OCP once. Very fast!

bernhard-42 commented 1 year ago

btw. , when you use the buttons "switch to iso view" and the "fit view", you should by back to the "reset" view

WhichQuord commented 1 year ago

I sometimes use the Jupyter approach also; I just prefer the full-run approach. Allowing some provision to override FIRST_CALL seems to support both approaches without one impacting the other. I wouldn't expect it to be a common use case to override it, but it's nice to support it if the user desires to do so.

I didn't think about "switch to iso view" followed by "fit view" accomplishing a reset, but I agree that's a sufficient solution. I'm not sure if it's just my installation, but I seem to get into the situation pretty often where "fit view" doesn't seem to do anything, so I haven't gotten into the habit of using it until now.

bernhard-42 commented 1 year ago

Allowing some provision to override FIRST_CALL seems to support both approaches without one impacting the other.

I need to think about whether your approach works in all corner cases (reset_camera is a bit of a nasty thing) or if I prefer to add (yet another) parameter or enum flag

"fit view" doesn't seem to do anything

Fit view simply sets the zoom factor back to 1. So if you haven't zoomed in or out, the zoom factor is 1 and "Fit view" does nothing. If the zoom factor is not 1 and "Fit view" does nothing, then it is a bug. You can check with status() or more explicit status()["zoom"] (this command asks the viewer about all current settings)

bernhard-42 commented 10 months ago

In 2.0.13 KEEP will retain the view across sessions. FIRST_CALL is removed. If one needs to have a reset for the first call, reset_camera=Camera.RESET needs to be added to show