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

Crop in pixel amounts instead of ratio #37

Merged
merged 1 commit into from
Aug 2, 2020

Conversation

e00E
Copy link
Contributor

@e00E e00E commented Aug 2, 2020

This allows exact pixel accurate cropping which is what users more
commonly want instead of using ratios.
The sliders for the crop amounts are hidden until open vr has been
initialized as we wouldn't know the maximum width and heigth values and
users wouldn't be able to see how their crop looks anyway.

Fixes #34

In my basic testing these changes work. Obs correctly remembers the previous crop values even when openvr is not yet running when obs starts up.

Feel free to make changes to the code yourself if you want to. Developing c++ with visual studio on windows is tedious for me so I'd rather be done with it and it is not important to me whether github credits me with the changes.

TODO:

  • Update the presets file with integer pixel values. Need to check what the current ratios map to.

Potential improvements:

  • Would it be better to use x and y offets (crop left and crop top) plus width and height instead of crop right and crop bottom?
  • As I mentioned in the issue there is a bug where the plugin does not update to the most recent property value when changing it rapidly. This can be observed by dragging the slider from some positive value to zero. The plugin internally will see for example 5 as the most recent value even though it should be 0. A user has to fix this by incrementing and decrementing the property again. To debug this I added the currently commented out comment printing the crop property values. I think this is a bug in OBS but I'm not sure. I definitely do not see how my changes caused this so the same bug should already be present in the current version and we can fix it in a separate PR.

Edit: I am debugging the latter issue with a small dummy plugin directly atm.

Comment on lines +37 to +42
struct crop {
unsigned int top;
unsigned int left;
unsigned int bottom;
unsigned int right;
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I refactored these into a struct because it is stored multiple times.

This allows exact pixel accurate cropping which is what users more
commonly want instead of using ratios.
The sliders for the crop amounts are hidden until open vr has been
initialized as we wouldn't know the maximum width and heigth values and
users wouldn't be able to see how their crop looks anyway.
@baffler baffler merged commit 9338cc6 into baffler:master Aug 2, 2020
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.

Allow setting crop in pixels instead of ratios
2 participants