Quelea icon indicating copy to clipboard operation
Quelea copied to clipboard

Projector Screen Margins

Open dallonf opened this issue 4 years ago • 12 comments

Adds an option to set margins for the projector in Display Settings:

image

As described briefly in https://github.com/quelea-projection/Quelea/issues/145, this is useful in cases where your projector projects beyond the physical boundaries of the screen and you need to change the shape so that the projection fits better onscreen. My church has a very extreme example where we can only lower the screen halfway without obscuring the band! (Margin Bottom 50%) After singing, we lower the screen the rest of the way. (Margin Bottom 0%)

You can already do this by tweaking the "Custom Position", but margins are a more convenient way to solve the problem. It's easier to remember 50% and 0% than 384 and 768 :)

There are some pretty serious problems I observed on Windows when your screens have different display densities (that is, DPI) - screens with margins often won't be sized correctly to the screen. I really don't know how to solve this and I think it would take a good deal more investigation to figure it out. Maybe for now, it'd be better to alert users somehow that there may be unexpected behavior, whether by documentation or an inline warning on the config screen. (maybe it can detect screens are different densities and show a message if the margins are non-zero?)

dallonf avatar Sep 07 '19 20:09 dallonf

In general this looks great to me! A few small notes however:

  • I personally would suggest using a slider instead of an integer field, I feel that would be easier to handle both from a developer's and user's perspective. If you look at the notice options, I created a custom percentage slider that might be useful in this scenario.
  • Codacy reports a few unused imports, could you perhaps delete them?
  • Regarding DPI, I've tried experimenting with it quite recently and to my knowledge you can get the value for a screen using double DPI = Screen.getScreens().get(PROJECTORSCREEN).getDpi(); In my case it returns 96 if no scaling is being used and 192 if it is scaled. I think you should be able to use that number to know if you should multiply the margin value or not. (Something like DPI > 100 ? marginTop * (DPI / 100) : marginTop;) Do you think that would work?

ArvidNy avatar Sep 08 '19 07:09 ArvidNy

I can make most of those changes you suggested! But I've done quite a bit of experimentation with DPI and whatever the fix is, it's not going to be simple or straightforward. It's not a problem specifically with margins, I think; even Custom Position behaves strangely in this situation. It seems to happen whenever the position and size of the projection window doesn't exactly match the full resolution of the screen (I suspect something, somewhere, is special-casing this specific use case of setting a window to cover an entire display, but even 1 pixel off confuses it).

I could probably investigate at some point in the future, but this has already been a pretty long project and I really need a break soon.

dallonf avatar Sep 10 '19 02:09 dallonf

Disregard the current commit for sliders - I just discovered PercentSliderControl which is closer to what I need, but is gonna need a little bit of refactoring to hook it up. I'll get to that in a few days.

dallonf avatar Sep 10 '19 02:09 dallonf

Sure, if that's the case for Custom Position too, I'd say a DPI solution could be an issue for a new PR at some point.

I still haven't tried running the code, but the changes look good to me from a code perspective anyways. No worries, we'll hold off merging this until you've switched the slider for a PercentSliderControl.

ArvidNy avatar Sep 10 '19 06:09 ArvidNy

Sorry for taking a long time to check this out! I've just tested the code and the custom position works just as expected. However, nothing happens if I use one of the Output options. I'm using a Linux system and if I remember correctly, Quelea for Linux has an extra full screen option that you might have to override as well. Would you mind checking that out as well?

ArvidNy avatar Sep 30 '19 14:09 ArvidNy

Hmm, good catch! Unfortunately I don't have a Linux development environment handy... not sure I'll be able to fix this anytime soon

dallonf avatar Sep 30 '19 15:09 dallonf

This is the method you need to take a look at. I did a quick test by returning false there and then it seems to work then. So if you only add an option to easily check if margins are used and then return false, it will work on Linux systems as well. Although perhaps you should enable setAlwaysOnTop anyways.

ArvidNy avatar Sep 30 '19 16:09 ArvidNy

Ok, when I'll get a free moment I'll make that change! I'll just need someone to test on Linux.

dallonf avatar Sep 30 '19 16:09 dallonf

If you make the change I could double-check that it works on Linux!

ArvidNy avatar Sep 30 '19 17:09 ArvidNy

@ArvidNy Ok, I did as much as I could without actually being able to test it!

I also notice that setFullScreenAlwaysOnTop function is called from VLCWindowDirect - does this need to be aware of margins as well? I don't have a lot of context on what this does.

dallonf avatar Oct 18 '19 00:10 dallonf

I'm not sure about VLCWindowDirect. I haven't noticed any issues while testing it anyways. @berry120, do you know if it needs to be aware of margins?

ArvidNy avatar Oct 18 '19 17:10 ArvidNy

Hey,

Sorry, just to say that we're aware this still needs looking at & approving, but we're currently a bit snowed under so it's likely to be after the 2020.0 release is out.

berry120 avatar Dec 10 '19 19:12 berry120