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

CS: Camera improvements #1582

Merged
merged 16 commits into from Dec 19, 2017

Conversation

Projects
None yet
3 participants
@drummyfish
Contributor

drummyfish commented Dec 7, 2017

I added a new preference section for Rendering (#1310) in which I put setting for camera FOV. Additionally I tried to fix an issue with orbiting camera mode which changed its roll and was practically unusable, at least to me. It now orbits only in yaw and pitch, while keeping the roll set with Q/E keys.

@zinnschlag

This comment has been minimized.

Show comment
Hide comment
@zinnschlag

zinnschlag Dec 8, 2017

Contributor

CS: Add rendering prefs and camera FOV

Hm, wondering. Would it make sense to have separate FOV settings for each camera mode?

CS: Make orbit camera not change roll

Maybe bind this feature to a new user setting (on by default)?

Contributor

zinnschlag commented Dec 8, 2017

CS: Add rendering prefs and camera FOV

Hm, wondering. Would it make sense to have separate FOV settings for each camera mode?

CS: Make orbit camera not change roll

Maybe bind this feature to a new user setting (on by default)?

@drummyfish

This comment has been minimized.

Show comment
Hide comment
@drummyfish

drummyfish Dec 8, 2017

Contributor

Hm, wondering. Would it make sense to have separate FOV settings for each camera mode?

I don't think that would really be useful for anything, can't see any use case, but you made me think maybe each separate 3D view could have a different FOV? That would make sense I guess. The FOV setting I've added now would be the default FOV of each new view, and then in each view you could adjust it differently (But how? Some key + mouse wheel? Or add a visual element for it?).

Also now when we're elaborating on this, I think I should also add the orthographic camera option, which could then also be toggled per-view. That would allow for really nice multi-view settings we know from 3D editors (e.g. this).

EDIT: But there's one catch for the orthographic projection: it's parametrized by view-size, which I'm not sure how to get - set it to some constant? That could be limiting. So I think it could be another preference setting, but that would also have to be per-view adjustable then.

Maybe bind this feature to a new user setting (on by default)?

Sorry I don't understand this, could you explain some more? Oh sorry, I get in now, my brain probably turned off for a while. Well I can do that I think, but I'd bet no one will ever turn it on :)

Contributor

drummyfish commented Dec 8, 2017

Hm, wondering. Would it make sense to have separate FOV settings for each camera mode?

I don't think that would really be useful for anything, can't see any use case, but you made me think maybe each separate 3D view could have a different FOV? That would make sense I guess. The FOV setting I've added now would be the default FOV of each new view, and then in each view you could adjust it differently (But how? Some key + mouse wheel? Or add a visual element for it?).

Also now when we're elaborating on this, I think I should also add the orthographic camera option, which could then also be toggled per-view. That would allow for really nice multi-view settings we know from 3D editors (e.g. this).

EDIT: But there's one catch for the orthographic projection: it's parametrized by view-size, which I'm not sure how to get - set it to some constant? That could be limiting. So I think it could be another preference setting, but that would also have to be per-view adjustable then.

Maybe bind this feature to a new user setting (on by default)?

Sorry I don't understand this, could you explain some more? Oh sorry, I get in now, my brain probably turned off for a while. Well I can do that I think, but I'd bet no one will ever turn it on :)

@drummyfish

This comment has been minimized.

Show comment
Hide comment
@drummyfish

drummyfish Dec 8, 2017

Contributor

Finding more bugs:

  1. When free camera is reset, the roll can't be changed.
  2. In orbit mode, when I get to limit top position with W key, the camera starts shaking and then if I press the S key, I sometimes "roll over the top" to the other side. fixed

(Don't know if it's just my branch, will test.)

Also is there any way to quickly start the editor with some cell loaded, for testing purposes?

Sorry for loads of text, I'm new to the editor, thanks for any help :)

UPDATE:

  1. Oh actually it's a feature of first person mode, sorry.
  2. My fault, gonna fix.
Contributor

drummyfish commented Dec 8, 2017

Finding more bugs:

  1. When free camera is reset, the roll can't be changed.
  2. In orbit mode, when I get to limit top position with W key, the camera starts shaking and then if I press the S key, I sometimes "roll over the top" to the other side. fixed

(Don't know if it's just my branch, will test.)

Also is there any way to quickly start the editor with some cell loaded, for testing purposes?

Sorry for loads of text, I'm new to the editor, thanks for any help :)

UPDATE:

  1. Oh actually it's a feature of first person mode, sorry.
  2. My fault, gonna fix.
@drummyfish

This comment has been minimized.

Show comment
Hide comment
@drummyfish

drummyfish Dec 8, 2017

Contributor

Another thing: the pref strings ("Rendering/camera-fov" etc.) are repeated over in multiple files so you have to make potential changes at multiple places => bug prone. Would be better to have a separate header with these pref string definitions.

Contributor

drummyfish commented Dec 8, 2017

Another thing: the pref strings ("Rendering/camera-fov" etc.) are repeated over in multiple files so you have to make potential changes at multiple places => bug prone. Would be better to have a separate header with these pref string definitions.

@drummyfish

This comment has been minimized.

Show comment
Hide comment
@drummyfish

drummyfish Dec 8, 2017

Contributor

I have the ortho camera now.

image

But I think 3D picking isn't working now, so that's the next thing. Actually works.

Contributor

drummyfish commented Dec 8, 2017

I have the ortho camera now.

image

But I think 3D picking isn't working now, so that's the next thing. Actually works.

drummyfish added some commits Dec 8, 2017

@drummyfish

This comment has been minimized.

Show comment
Hide comment
@drummyfish

drummyfish Dec 8, 2017

Contributor

So I've added the option to turn on the old orbit behavior, which is off by default. I'm kinda happy with the camera now.

Contributor

drummyfish commented Dec 8, 2017

So I've added the option to turn on the old orbit behavior, which is off by default. I'm kinda happy with the camera now.

@drummyfish

This comment has been minimized.

Show comment
Hide comment
@drummyfish

drummyfish Dec 10, 2017

Contributor

Could maybe the parameter settings for individual views look like this? (one dial for FOV, another for ortho projection size and a button for perspective/ortho mode - the button would actually be better placed before the dials)

image

Contributor

drummyfish commented Dec 10, 2017

Could maybe the parameter settings for individual views look like this? (one dial for FOV, another for ortho projection size and a button for perspective/ortho mode - the button would actually be better placed before the dials)

image

@zinnschlag

This comment has been minimized.

Show comment
Hide comment
@zinnschlag

zinnschlag Dec 11, 2017

Contributor

Nope. That would mix up modes and mode settings. Do we actually need this in the scene UI. I thought the regular user settings UI would be doing fine. I can't come up with a use case where you would want to change FOV settings on the fly.
However if there are use cases that I missed, the right way to do it would be via the button context menu. There are a couple of examples with other buttons on how to do this.

Contributor

zinnschlag commented Dec 11, 2017

Nope. That would mix up modes and mode settings. Do we actually need this in the scene UI. I thought the regular user settings UI would be doing fine. I can't come up with a use case where you would want to change FOV settings on the fly.
However if there are use cases that I missed, the right way to do it would be via the button context menu. There are a couple of examples with other buttons on how to do this.

@drummyfish

This comment has been minimized.

Show comment
Hide comment
@drummyfish

drummyfish Dec 11, 2017

Contributor

Nope. That would mix up modes and mode settings.

Yep I thought that would be a problem, I just couldn't come up with anything else. I'm scratching that.

Do we actually need this in the scene UI. I thought the regular user settings UI would be doing fine.

You mean the preferences that I have now? Yes, they're good for setting the default values, but I was still thinking about having the additional ability to adjust the parameters per each opened 3D view. I guess I'm still thinking about it from 3D editor point of view - in Blender I usually have multiple views and keep changing settings in each one separately on the fly based on what I'm doing, e.g. I might do view1: top ortho, view2: side ortho, view3: perspective. This way I can do very precise aligning and selection and have very high space awareness. Then I may close the first two views and keep only the perspective in bigger resolution to preview the scene. But:

  • I don't know if how I'm working with Blender is how CS is supposed to work, it's a different type of program.
  • I might leave it for another PR if there is discussion to be had, for example about the exact GUI for this.

I can't come up with a use case where you would want to change FOV settings on the fly.

Changing FOV on fly may be rarely used, but changing between ortho and perspective would be pretty frequent I imagine, so for completeness I would simply allow all (3) camera parameters that can be set in preferences to be also changed per-view on-the-fly, if we were to do that.

Contributor

drummyfish commented Dec 11, 2017

Nope. That would mix up modes and mode settings.

Yep I thought that would be a problem, I just couldn't come up with anything else. I'm scratching that.

Do we actually need this in the scene UI. I thought the regular user settings UI would be doing fine.

You mean the preferences that I have now? Yes, they're good for setting the default values, but I was still thinking about having the additional ability to adjust the parameters per each opened 3D view. I guess I'm still thinking about it from 3D editor point of view - in Blender I usually have multiple views and keep changing settings in each one separately on the fly based on what I'm doing, e.g. I might do view1: top ortho, view2: side ortho, view3: perspective. This way I can do very precise aligning and selection and have very high space awareness. Then I may close the first two views and keep only the perspective in bigger resolution to preview the scene. But:

  • I don't know if how I'm working with Blender is how CS is supposed to work, it's a different type of program.
  • I might leave it for another PR if there is discussion to be had, for example about the exact GUI for this.

I can't come up with a use case where you would want to change FOV settings on the fly.

Changing FOV on fly may be rarely used, but changing between ortho and perspective would be pretty frequent I imagine, so for completeness I would simply allow all (3) camera parameters that can be set in preferences to be also changed per-view on-the-fly, if we were to do that.

@zinnschlag

This comment has been minimized.

Show comment
Hide comment
@zinnschlag

zinnschlag Dec 12, 2017

Contributor

Not entirely convinced that this is practical, but I don't have any objections.

Contributor

zinnschlag commented Dec 12, 2017

Not entirely convinced that this is practical, but I don't have any objections.

@drummyfish

This comment has been minimized.

Show comment
Hide comment
@drummyfish

drummyfish Dec 12, 2017

Contributor

We might still leave it for later though, I just found that I may not have much time in the following days and I'd like to ask away at forums first anyway, so let's keep it this way maybe. Should I do anything else for this PR then? What do you think about the code?

Contributor

drummyfish commented Dec 12, 2017

We might still leave it for later though, I just found that I may not have much time in the following days and I'd like to ask away at forums first anyway, so let's keep it this way maybe. Should I do anything else for this PR then? What do you think about the code?

@zinnschlag

This comment has been minimized.

Show comment
Hide comment
@zinnschlag

zinnschlag Dec 13, 2017

Contributor

Looks okay, but I will give it a more thorough review within the next few days.

Contributor

zinnschlag commented Dec 13, 2017

Looks okay, but I will give it a more thorough review within the next few days.

@zinnschlag

This comment has been minimized.

Show comment
Hide comment
@zinnschlag

zinnschlag Dec 15, 2017

Contributor

Gave it another test-drive. Looks okay. But there is still some room for improvements in regards to the user settings.

  • Do we really need doubles for the numeric values? Looks unnecessary cluttered IMO. Plus, I don't remember ever seeing a FOV slider that allowed changes of 0.1 or even smaller.
  • Should there be a tooltip for "Orthographic projection size parameter? Not sure about this one.
  • The 3d Scene Input page is starting to look a bit messy. Actually had to look into the source to locate the new option. Maybe sorting options by camera type would help. Or splitting camera controls into a separate page?

btw. just noticed that there is an "Object Marker Transparency" option in "3D Scene Input" that really belongs on your new Rendering page instead. Not really part of this task, but while your at it could you move it over?

Contributor

zinnschlag commented Dec 15, 2017

Gave it another test-drive. Looks okay. But there is still some room for improvements in regards to the user settings.

  • Do we really need doubles for the numeric values? Looks unnecessary cluttered IMO. Plus, I don't remember ever seeing a FOV slider that allowed changes of 0.1 or even smaller.
  • Should there be a tooltip for "Orthographic projection size parameter? Not sure about this one.
  • The 3d Scene Input page is starting to look a bit messy. Actually had to look into the source to locate the new option. Maybe sorting options by camera type would help. Or splitting camera controls into a separate page?

btw. just noticed that there is an "Object Marker Transparency" option in "3D Scene Input" that really belongs on your new Rendering page instead. Not really part of this task, but while your at it could you move it over?

@drummyfish

This comment has been minimized.

Show comment
Hide comment
@drummyfish

drummyfish Dec 15, 2017

Contributor

I tried to address some of these issues. However, does declareSeparator() do anything? I can't really see any separators in the menu. (Or should I use declareSubcategory?) Also should I make near and far clip planes configurable as well? (I already tried but changing the values somehow did nothing for some reason, don't know if I should continue the effort.)

Contributor

drummyfish commented Dec 15, 2017

I tried to address some of these issues. However, does declareSeparator() do anything? I can't really see any separators in the menu. (Or should I use declareSubcategory?) Also should I make near and far clip planes configurable as well? (I already tried but changing the values somehow did nothing for some reason, don't know if I should continue the effort.)

@zinnschlag

This comment has been minimized.

Show comment
Hide comment
@zinnschlag

zinnschlag Dec 16, 2017

Contributor

However, does declareSeparator() do anything?

Introduces are small vertical gap. Maybe it is a bit too subtle.

Also should I make near and far clip planes configurable as well? (I already tried but changing the values somehow did nothing for some reason, don't know if I should continue the effort.)

Not sure. @scrawl Any input?

Contributor

zinnschlag commented Dec 16, 2017

However, does declareSeparator() do anything?

Introduces are small vertical gap. Maybe it is a bit too subtle.

Also should I make near and far clip planes configurable as well? (I already tried but changing the values somehow did nothing for some reason, don't know if I should continue the effort.)

Not sure. @scrawl Any input?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Dec 18, 2017

OSG autocomputes near/far by default to match the current viewport. You could turn that off and use manual values, but I don't see the point tbh.

ghost commented Dec 18, 2017

OSG autocomputes near/far by default to match the current viewport. You could turn that off and use manual values, but I don't see the point tbh.

@drummyfish

This comment has been minimized.

Show comment
Hide comment
@drummyfish

drummyfish Dec 18, 2017

Contributor

Okay, I'd leave it at that. So anything else here? :)

Contributor

drummyfish commented Dec 18, 2017

Okay, I'd leave it at that. So anything else here? :)

@zinnschlag zinnschlag merged commit 01f9d90 into OpenMW:master Dec 19, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment