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

make reset_camera=Camera.KEEP the default behavior #144

Open
jdegenstein opened this issue Jan 26, 2025 · 8 comments
Open

make reset_camera=Camera.KEEP the default behavior #144

jdegenstein opened this issue Jan 26, 2025 · 8 comments

Comments

@jdegenstein
Copy link
Contributor

jdegenstein commented Jan 26, 2025

Resetting the view direction on re-running show is very disruptive to my workflow since it often requires me to take one hand off the keyboard and re-orient the camera manually. To address this concern the Camera.KEEP option was added previously, I propose that this becomes the default behavior.

@bernhard-42
Copy link
Owner

bernhard-42 commented Jan 26, 2025

I don't think this is a good idea. That'll be totally confusing for new users:

set_defaults(reset_camera=Camera.KEEP)

show(Pos(1, 1, 1) * Box(1, 2, 3))
# %%

show(Pos(-2, -2, -2) * Sphere(1))

You can't see the sphere. New users might wonder.

I wonder how hard it is to set_defaults(reset_camera=Camera.KEEP) at the top of your fine?

@snoyer
Copy link

snoyer commented Jan 26, 2025

If my understanding is correct the issue is not really "reset bad, keep good" or the other way around but "don't overrule the user", in which case part of the solution would need to be on the viewer side with the UI keeping track of whether the camera has been changed manually or not.

Then you could have some logic (new parameter? different semantics for KEEP values? checkbox on the viewer side?) that would allow automatic resetting (@bernhard-42's point) unless the user has modified the camera (@jdegenstein's point).

(obviously this is very handwavy and there would be further points to address such as subsequent show calls knowing whether they are running a new script or the previous one again and whatnot)

@bernhard-42
Copy link
Owner

This is not about bad or good. I simply don't want to get "Viewer is broken" issues because of the behaviour described above (sphere not visible because of KEEP). Hence, the viewer's default is RESET.
Jern's use case is exactly why set_defaults exist. It allows for pro users to set KEEP for the scope of the file they are working on - which most probably deals with working on one object where Keep makes sense.

And, I don't want to make an already complex logic not even more complex.

@snoyer
Copy link

snoyer commented Jan 26, 2025

@bernhard-42 I agree with your argument; I also agree with @jdegenstein's but I don't think having Camera.KEEP as the default would fully solve the problem (for the reason you pointed out) even if it was an acceptable change.

Personally what I like about show() is how unobtrusive and agnostic is feels, I just need from ocp_vscode import show and that's it. It would be great if the default behavior allowed to both automatically framing the expected objects the first time around (as it does now) but also not reset every subsequent time re-running the same script after tweaking it.

I admit my earlier suggestion for "don't reset if the user has touched the camera manually" is probably over complicated, maybe there is a simpler middle-ground solution... something like "don't reset if the objects are (roughly) in the same place" based on some kind of overall bounding box heuristic? Assuming there's interest for such an improvement (if it is an improvement at all) that is.

And, I don't want to make an already complex logic not even more complex.

"not a bug, won't fix" is also a valid take :D in that case I'll stop wasting your time

@jdegenstein
Copy link
Contributor Author

First off, thank you for the thoughtful responses and discussion so far. Responding to a few points raised above:

I wonder how hard it is to set_defaults(reset_camera=Camera.KEEP) at the top of your fine?

You are correct. It is not hard, but that requires users to know about this setting (and there are a myriad of options for set_defaults and show itself).

And, I don't want to make an already complex logic not even more complex.

RE: some of @snoyer 's suggestions and this point from bernhard, I completely agree with bernhard on this point.

Given this discussion so far I decided to consult with the defacto most popular CodeCAD user interface -- OpenSCAD (note: I am not saying we SHOULD consider anything below as "sacred" or "perfect" but I think it is worthwhile to research other software as inspiration because UI generally comes with a heritage that usually conforms to user expectations/experience). Here are a few relevant observations:

  1. running translate([40,0,0]) cube(3); + F5 (preview) results in a cube that is off screen in the default zoom level + view orientation of "diagonal" (roughly similar to isometric).
  2. panning the viewer off the default view to view the cube and then hitting F5 again, results in the viewer NOT resetting to the default orientation (similar to Camera.KEEP in OCPCV)
  3. OpenSCAD seems to never alter the viewer camera direction/position/zoom without some user input (similar to Camera.KEEP in OCPCV)
  4. In OpenSCAD pressing View -> Reset View seems to always reset the viewer to the default orientation, zoom level, and point at the global origin.
  5. starting at (1) above and using OpenSCAD menus View -> View All results in the viewer (a) automatically point at the cube (b) camera panned from where it started (c) camera "zoomed in" to the cube so it takes up a large percentage of screen space. What I am wondering is should there be an analogous button in the OCPCV viewer user interface that performs the same exact function? This is important because it should solve the stated user concern from @bernhard-42 of "not being able to see what I just modeled".

I also want to say that I fully agree with snoyer that some version of "won't fix" is a totally acceptable answer (this is just a friendly suggestion as a user of this software). I am obviously an avid user of Camera.KEEP but before it existed I really struggled to switch off of CQ-editor because I at least felt that the viewer wasn't "fighting me". This was sadly in spite of CQ-editor's lack of critically important modern editor features.

@bernhard-42
Copy link
Owner

@jdegenstein I really like your idea of looking into what OpenSCAD does. Thanks for the idea.

I also checked it out.

  • As the first command, translate([50,0,0]) cube(3); <F5> not shown in my OpenSCAD viewer, had to press "Show All"
  • I then did translate([0,0,0]) cube(3); <F5>, and again nothing is shown, had to press "Show All" again.

Now, OCP CAD Viewer Camera.RESET does exactly that: "show the object(s)" and then "Show All".
I think I prefer this behaviour as default.

Now, let's address the point of set_defaults(reset_camera=Camera.KEEP not being hard, but one needs to know.
Very fair point.

Two proposals (both to implement):

  1. Create a proper docs page - should have done that long ago, but there is always so much to develop :-)
  2. Have a button next to "Ortho" in the viewer that when you press it keeps the Camera as is and when you press it again (deactivate) resets the view. It will be overwritten by reset_camera= in the show command, just as the "Ortho" button.

I think 2) solves two problems:

  • Discoverability
  • When I found the right angle and zoom, I don't need to change code adding resetCamera=, but can simply press the camera button and continue with my code.

Of course, set_defaultswill still work and set the button and the view like ortho= does.

What do you think?

@jdegenstein
Copy link
Contributor Author

Now, OCP CAD Viewer Camera.RESET does exactly that: "show the object(s)" and then "Show All".
I think I prefer this behaviour as default.

Not my preference, but this is your project. (just to recap: I prefer that the viewer by default never moves the camera upon "re-render" without separate user input (like OpenSCAD)).

Two proposals (both to implement):

These are great and will go a long way to improving the user experience. I also like the interaction between set_defaults and viewer buttons.

I think it is also worth reviewing these two buttons to see if they are still relevant given your proposal. I don't have a strong preference on how they should act but they seem related to this.

Image

@bernhard-42
Copy link
Owner

They are still relevant. The left button will return camera angles and zoom to where you started and the right one will keep camera angles and only zooms to fit the window. I use both very regularly.

just to recap: I prefer that the viewer by default never moves the camera upon "re-render" without separate user input

I can additionally make reset_camera a workspace setting, but the default will be RESET. You would need to change it once, but if you never press the reset button it will forever be KEEP

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

No branches or pull requests

3 participants