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

Canvas Rescaler #1781

Merged
merged 14 commits into from
Mar 23, 2017
Merged

Canvas Rescaler #1781

merged 14 commits into from
Mar 23, 2017

Conversation

koosemose
Copy link
Collaborator

This is an alternative possible fix for #471. Rather than having to fiddle with different settings on various UI components, this simply watches for a change in the width of the screen and changes the scaling of the UI to match so that the UI is larger on larger displays and smaller on smaller.

This seems to produce good looking results for both larger and smaller screens, but I can't be absolutely sure since I can only test on one monitor size and that won't accurately reflect how it will look for someone with actual different resolution monitors.

This is just a rough proof of concept and still needs some cleanup. But if it produces the desired effects it can allow for an easy place to plug-in giving the user some control over the scale as well, so that it can be adjusted beyond just automatic scaling.

Ping: @bjubes (for testing on his ultra monitor) @BraedonWooding (since the testing and back and forth on various pros and cons on different approaches on his PR to fix this same problem inspired this)

@BraedonWooding
Copy link
Collaborator

BraedonWooding commented Feb 18, 2017

I'm perfectly fine with this (I haven't tested it just the idea), I don't really have the time to implement a proper fix for the UI and originally my PR was mainly just fix this and that, then became a little too band-aid fixey for its own good lol.

@bjubes
Copy link
Collaborator

bjubes commented Feb 21, 2017

It now looks really good on 5K mac:
screen shot 2017-02-20 at 8 28 45 pm
The only problem is that it had the opposite effect (for me) on my normal 1080 14inch laptop...
capture

This makes it seem that the solution should use dpi with/instead of resolution, since this just inverts the problem.

@bjubes
Copy link
Collaborator

bjubes commented Feb 21, 2017

whats even weirder is that these images look the same, so I think that supports this being a DPI problem not just a pixel resolution one.

@koosemose
Copy link
Collaborator Author

Yeah, the intent was that they the scaling would lead to them looking the same, I may have slightly overdone it, but the size at 1080p, is basically the "native" size of things.

Since (to the best of my knowledge) we can't get dpi info, I think this is where user UI scaling comes in. I tweak the numbers a bit to get acceptable results across a decent range, then the user can tweak the UI scale to make it perfect for themselves.

}
}

private IEnumerator AdjustScale()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this a coroutine? This will behave just like a function the way you have implemented it, since nothing happens after or while it yields.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No reason, other than leftovers... it used to do something, but I was tweaking all sorts of things trying to get things to behave how I wanted, and I forgot to uncoroutine it.

@koosemose
Copy link
Collaborator Author

If you could give this another test, it'd be appreciated... I've tried doing a dpi based scaling... but I feel like it won't work as intended... but doing it the other way around doesn't seem to make sense either... but I may just be having logic failures..

@koosemose koosemose mentioned this pull request Feb 25, 2017
@koosemose
Copy link
Collaborator Author

@SquareMan If you could give this a test, it would be appreciated.

The expected result is that the UI will still be too small as @bjubes reported, if not in fact worse. The desired result is that the UI will be at a reasonable (and readable) size, but I don't have high hopes that the current change will affect that.

@bjubes
Copy link
Collaborator

bjubes commented Mar 4, 2017

Retested. Its still just as small on high DPI 1080p 13ish inch screen

@koosemose
Copy link
Collaborator Author

As best as I can tell from the research I've been doing trying to figure out the right terminology to use in figuring out how to solve the problem, if it's the exact same size, that suggests the monitor isn't actually high dpi, just the standard 96 dpi (which is what I've used as the baseline since research suggested that is the most common dpi), or that it's not properly reporting dpi, in which case it assumes 96, since it's the most common, and any other methods I've seen for getting dpi are error prone in a way that would lead to things being unpredictable, and I'd prefer failure in a predictable way rather than unpredictable.

I think what I'd need access to to get it perfectly correct would be ppi (pixels per inch) rather than dpi, which is more on the hardware side. Unfortunately, as best I can figure (from digging around in Screen) I can't get PPI, or anything that would allow me to calculate it.

So I think the best solution, will be to rely primarily on manual scaling for the more extreme ends. I think it could also do with the baseline tending towards slightly larger UI, as I suspect things on the smaller end will be more common than the larger end, and I suspect the UI being too large on a larger higher resolution screen will be less negative than being too small and possibly hard to read on a smaller screen.

@BraedonWooding
Copy link
Collaborator

I agree, a UI scaling system will be a much stronger option for extremes, since an automatic one will always be less preferable in some cases.

@koosemose
Copy link
Collaborator Author

Removed the dpi scaling because I don't think it was doing what I wanted. Added controls in settings to adjust UI scaling (currently in localization because I simply just dropped it in wherever, and as I don't think it's a video option, more of a generic graphics sort of option, and not absolutely sure if I should just create a graphics setting under general or lump it under video despite not thinking it's technically a video option, will move it later pending input from others).

I also adjusted the screen size scaling to be based on height rather than width, which may have been causing it to be undersized on non widescreen ( @bjubes if your smaller monitor is non-widescreen this should either fix the issue, or at least reduce the problem, if it continues to be a problem, I can further adjust the baseline, but I've been trying to keep the baseline sizes based on standard resolutions, and baseline being 720p leads to, what I feel is a little too large. But if it continues to be a problem I can either just deal with the oversized, or make the baseline in between the two... or explore different solutions).

@koosemose
Copy link
Collaborator Author

koosemose commented Mar 6, 2017

A few more touches, mostly to only apply scale when it has changed (was originally done when screen size changed, but that was temporarily commented out for testing the ui scale setting, now it is back to doing it on screen size change and changing of the scale setting). I am a little bit torn on rather or not to cache a reference to the UIRescaler (to access the apply scaling function). On the one hand it could be argued that it would be proper to cache it if it is going to be accessed multiple times, but on the other hand, it isn't going to be used all that often, and even if it is, it is still right in the middle of a lot of other operations which will have a more noticable effect on performance, so I think the negligible savings in performance is outweighed by (in a negligible way) the negligible savings in memory.

Finally the slider has been made to only do actual steps of .1 (previously, despite displaying only the .1 intervals, it could have further precision beyond this, which seemed sloppy, in that one could have a bit of wiggle of sizes around any given number. I am not completely satisfied with the method of doing this (setting to use wholenumbers, and dividing/multiplying the number as needed), but I think it is superior to the alternatives (either trying to code up some manual .1 step locking in the valueChanged override, or just translating whater number was resulted to something on the .1 step).

Additionally there is one bug that shows in the Settings dialog, but I don't think it is technically a bug in this code (not in the Settings dialog either), just that settings dialog was coded with the UI at the time in mind (set size, not scaling based on outside factors), and just needs tweaked to account for this a bit. I notice it start appearing on 720p and smaller, where the two columns will be overlapped (in addition to the text getting fuzzy on extremely smaller resolutions, which the other text does not get similarly fuzzy). It will also do it with smaller manual scales. At a guess, the settings dialog may be trying to do its own scaling which creates a bit of chaos when stacked with another scaling method.

Correction: Actually quite a few subelements of various dialogs aren't handling the scaling properly, so I will have to investigate this further.

Addendum to Correction: The different dialogs are misbehaving in different ways (though with similarities due to them all being some reaction to scaling), so it will take a lot more investigation than initially thought.

@koosemose koosemose changed the title [WIP] Canvas Rescaler - Needs testing at multiple resolutions Canvas Rescaler Mar 16, 2017
@koosemose
Copy link
Collaborator Author

Ok, so I've got a functional (though not entirely satisfactory) solution to the weird scaling of UI elements, it seems to be due to an odd little bug (or perhaps odd design) in the Unity UI system when scaling of the canvas is involved, where subelements get scaled strangely if created after canvas scale is changed. The workaround is to simply manually set localScale to 1 on these subcomponents. As it seemed a slightly better solution to me, I've made a quick little script to attach to any misbehaving UI elements that just sets localScale to 1 in Start, rather than having to dig through the code for everywhere a given UI component is instantiated and applying the scaling there.

There is of course also the possibility that I have overlooked a few elements that need their scaling fixed, but I got all the ones I could find/think to look for. There were also some scaling issues which I got sorted out (most notable example being the Settings menu, which scaled oddly before, overall staying the same size, but with certain things, such as the text on buttons scaling larger or smaller relative to the button itself. As far as I can identify behavior should remain basically the same, staying the same apparent size, minus the undesirable effects.

@BraedonWooding
Copy link
Collaborator

Just want to add the Setting menu has a custom scaling system, so it may be notable to move it to a separate canvas, this was mainly so more tight control could be had over the elements themselves and so it would favour scrollbars in some situations over really small elements. Other than that your solution looks brilliant 👍

@koosemose koosemose merged commit 25cd5d9 into TeamPorcupine:master Mar 23, 2017
@koosemose
Copy link
Collaborator Author

Everything seems to work as intended, other than some remaining issues with Settings, which I will leave to Braedon, as he seems to have confidence in sorting it out. There may also be UI elements that I overlooked, but for the most common issues, there is a script in place that will just need to be applied, and I can't wait around forever on the potentiality that I overlooked some UI. Merged.

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.

3 participants