-
Notifications
You must be signed in to change notification settings - Fork 149
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
Projector Screen Margins #190
base: master
Are you sure you want to change the base?
Projector Screen Margins #190
Conversation
# Conflicts: # Quelea/languages/gb.lang # Quelea/src/main/java/org/quelea/windows/options/OptionsDisplaySetupPanel.java # Quelea/src/main/java/org/quelea/windows/options/SingleDisplayPanel.java
In general this looks great to me! A few small notes however:
|
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. |
Disregard the current commit for sliders - I just discovered |
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 |
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? |
Hmm, good catch! Unfortunately I don't have a Linux development environment handy... not sure I'll be able to fix this anytime soon |
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 |
Ok, when I'll get a free moment I'll make that change! I'll just need someone to test on Linux. |
If you make the change I could double-check that it works on Linux! |
…ettings-ui # Conflicts: # Quelea/languages/gb.lang
@ArvidNy Ok, I did as much as I could without actually being able to test it! I also notice that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good to me, there's just one small thing I'd want you to change before I'd be willing to merge it (see separate comment).
There also are some issues with the video window not always getting resized properly, but I noticed that we have that issue with custom position screens as well, so I'd say we could live with that for now and sort it out later.
The only other thing that might be an issue is that the projection window isn't set to be always on top with your code. @berry120, do you think that's needed before we can merge it or should we leave it out for now?
} | ||
fiLyricWindow.setAreaImmediate(bounds); | ||
if (!QueleaApp.get().getMainWindow().getMainPanel().getLivePanel().getHide().isSelected()) { | ||
fiLyricWindow.show(); | ||
} | ||
|
||
// non-custom positioned windows are fullscreen | ||
if (!QueleaProperties.get().isProjectorModeCoords()) { | ||
if (!QueleaProperties.get().isProjectorModeCoords() && !QueleaProperties.get().hasProjectorMargin()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line keeps the projection window from closing if neither a custom position is set nor an output, whereas margins are set. I would suggest moving it to the line below in the else statement where it's actually needed:
} else {
if (!QueleaProperties.get().hasProjectorMargin()) {
fiLyricWindow.setFullScreenAlwaysOnTop(true);
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! Updated
I'm not sure about |
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. |
Adds an option to set margins for the projector in Display Settings:
As described briefly in #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?)