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

[Editor / Launcher] UI fixes and theme enhancements #667

Merged
merged 5 commits into from
Apr 30, 2020
Merged

[Editor / Launcher] UI fixes and theme enhancements #667

merged 5 commits into from
Apr 30, 2020

Conversation

RayKoopa
Copy link
Contributor

@RayKoopa RayKoopa commented Apr 24, 2020

PR Details

This PR fixes or updates the look-and-feel of some UI elements which seem blurry or outdated.

Description

  • Fixing blurry controls (excluding inconsistent toolbar / menu content for now as I may handle that in a separate PR).
  • Updating the window chrome to look like in Blend / VS2019:
    • New caption buttons, new window icon (filled Stride icon)
    • Allow clicking the window icon by moving the mouse to the top left corner when maximized.
    • Allow closing a window by double clicking the window icon.
    • Minimize and Maximize buttons are hidden if Window.ResizeMode is set to NoResize.
    • Span background image of Launcher into title bar for a "modern" look.
  • Updating the scroll bars to look like in Blend / VS 2019.
  • Updating some colors to be more in line with the Blend 2019 theme.
  • Replacing some rasterized graphics with vector ones (like the Stride logo in the About dialog) to also fix colorization with the new light / dark themes.
    • About dialog
    • Launcher dialog? (postponed, will do in later PR, missing vector sources yet)

Related Issue

None

Motivation and Context

Controls should not be blurry throughout the Launcher and Editor / Game Studio. Images should not pixelate on high DPI. Colors should not make text hard to read (blue highlight is too bright and mismatches Blend theme, especially affecting white text on it).

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation. (Screenshots may need updating)
  • I have added tests to cover my changes.
  • All new and existing tests passed.

- Globally enable snapping subpixel elements to pixel grid to not make them smear between two pixels.
- AvalonDock windows additionally require layout bounds to be snapped to pixel grid as they would still cause blurred controls otherwise.

Does not fix (bitmap) icons which are rendered at a size not matching their native resolution; I will attempt to fix this later by updating / replacing icons and optimizing menu / toolbar rendering / layouts.
@CLAassistant
Copy link

CLAassistant commented Apr 24, 2020

CLA assistant check
All committers have signed the CLA.

@tebjan
Copy link
Member

tebjan commented Apr 24, 2020

Cool, thanks for the good work! Especially the extensive commit message. :)

From what I read, this PR seems to try to do a lot of things at once, I think it could be broken up into a few separate ones. If the first two commits already improve something, there is no need to wait for the other features artificially.

I've also seen screenshots from your work on Discord, maybe add them here as well.

@RayKoopa
Copy link
Contributor Author

I also thought of creating separate PRs, but eventually decided to put it into one, as "fixing blurry things" spans multiple parts (not only snapping controls to pixel grid, but also updating blurry caption bar glyphs, and thus updating the title bar, and thus updating the theme slightly to be in line with Blend...).

I'll add screenshots soon as I ported the changes over to the current master (need to recheck things after the new theming feature was added yesterday).

@tebjan
Copy link
Member

tebjan commented Apr 24, 2020

Yes, that makes sense! Looking forward to the next commits :)

@Tonedus
Copy link

Tonedus commented Apr 24, 2020

The issue of UI improvements shouldn't be underestimated, thank you for bringing this up, I look forward to examples!

- New editor icon (filled logo).
- New caption button glyphs.
- Double clicking the window icon now closes the window.
- Slightly adjusted theme colors.
- Make PrivacyPolicyWindow non-resizable. Removes unnecessary caption buttons and fixes bug with with window chrome showing black borders at the right and bottom.
- Modify / add app manifests to use consistent DPI scaling. Also use Windows visual styles; system message boxes look better then (one appears when declining the privacy policy window).
@RayKoopa
Copy link
Contributor Author

Here are some WIP screenshots.

  • The background brush of windows is now extended into the title bar, which makes the Launcher image span all of its window.
  • If you want a background brush to only apply to the client area (like before), define a border spanning all of the window and having that brush set, and wrap your window contents in it.
  • I already discussed some Launcher color changes on Discord, which I want to handle in a separate PR following later.
    image

Here are some editor screenshots of the new themes and how they play out with the caption bar for different windows. Note that slight blurryness of controls (especially noticeable on those which have borders) is also fixed.

  • "Expression Dark" (practically Blend, darker than VS 2019 Dark, default)
    image
  • "Dark Steel" (meant as a theme like Unity / Unreal. Brighter than VS 2019 Dark). Note that minimize and maximize buttons are no longer visible if the window is not resizable.
    image
  • "Light Steel Blue" (practically VS 2019 Light, currently experimental and missing several vectorized icons to recolor them)
    image

- Removes the Vista/7-esque fade animations (goodbye storyboards).
- Also suffixes the brushes I introduced in the last commit with "Brush" to make their keys unique.
@RayKoopa
Copy link
Contributor Author

RayKoopa commented Apr 24, 2020

Does anyone know if vector versions of these icons exist somewhere? I only found PNGs in the repository, and I'd like to use vectorizations here to prevent pixelation on high DPI (and recolor support for lighter themes later on). I removed that task for this PR.
image

@RayKoopa RayKoopa changed the title [WIP] UI fixes and theme enhancements UI fixes and theme enhancements Apr 24, 2020
@RayKoopa RayKoopa marked this pull request as ready for review April 24, 2020 18:36
@RayKoopa RayKoopa changed the title UI fixes and theme enhancements [Editor / Launcher] UI fixes and theme enhancements Apr 24, 2020
@tebjan
Copy link
Member

tebjan commented Apr 24, 2020

for my eye this gray is a bit too bright... how about:
Hex: 252526
or
Hex; 1E1E1E

which are both darker visual studio grays...

image

@RayKoopa
Copy link
Contributor Author

"Dark Steel" is not meant to be a VS-like theme; it is meant to be something like Unity / Unreal.
Definitely, there is some outstanding discussion about which themes to simulate and which additional ones should be there.

@xen2 xen2 marked this pull request as draft April 24, 2020 21:56
@xen2
Copy link
Member

xen2 commented Apr 24, 2020

@RayKoopa nice PR, thanks!

Since you mentioned some areas are still in progress, I converted PR status as "draft".
Please use the "Ready for review" button when it's complete.

@RayKoopa
Copy link
Contributor Author

RayKoopa commented Apr 24, 2020

@xen2 The only thing I originally planned for this PR was to update the Launcher icons to become vectorized, due to missing sources I postponed that.
Or do you mean the Checklist? I'm not fully sure how to create and run tests for these UI changes other than manually checking everything looks correct at runtime, also because I currently cannot build most test projects due to build errors.

Of course, if there's something I mentioned for another PR which should be in this one before merging it, let me know!

@Basewq
Copy link
Contributor

Basewq commented Apr 24, 2020

What xen2 means that if you're still adding commits to this PR, then it's better to leave it in draft mode so he doesn't try to merge this, since you might end up changing stuff while he's merging obsoleting code.
You can change it back to a full PR when you think this particular PR is "done" (ie. you think it can be merged as is).

@RayKoopa
Copy link
Contributor Author

Yep, I'm done pushing to the branch I'm PRing from; that's why I marked it as ready for review.

@Basewq
Copy link
Contributor

Basewq commented Apr 24, 2020

Ah, I suppose xen2 got confused on the timeline. You mentioned WIP, but you did mark for review later, but you didn't actually comment that you're done so he probably missed that.
I guess since you've explicitly mentioned you're done now, you can mark it back for review again.

@RayKoopa
Copy link
Contributor Author

Ah sorry about it then. I thought he meant the postponed changes I would like to handle in later PRs as I don't know for how long they'll be postponed.

Everything checked off at the top is done! This PR is ready to be merged from my side.

@xen2 xen2 marked this pull request as ready for review April 25, 2020 10:29
@xen2
Copy link
Member

xen2 commented Apr 25, 2020

@RayKoopa OK understood, sorry for the misunderstanding. Maybe I didn't see the part where you said "I removed that task for this PR." for the vector icons.

@xen2 xen2 requested a review from Kryptos-FR April 25, 2020 10:32
@RayKoopa
Copy link
Contributor Author

Alright, I try to be more clear next time :D

@xen2
Copy link
Member

xen2 commented Apr 25, 2020

@RayKoopa No worries, it was my fault :)

@xen2
Copy link
Member

xen2 commented Apr 25, 2020

I asked @Kryptos-FR for a review, then LGTM (conceptually) for merge.

@Kryptos-FR
Copy link
Member

@xen2 @RayKoopa Looking at it...

Copy link
Member

@Kryptos-FR Kryptos-FR left a comment

Choose a reason for hiding this comment

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

I am a bit concerned about the extensive use of dynamic resource binding.

sources/editor/Stride.GameStudio/app.manifest Show resolved Hide resolved
sources/launcher/Stride.Launcher/app.manifest Show resolved Hide resolved
<Border x:Name="ContentBorder" Background="{TemplateBinding Background}" BorderBrush="{DynamicResource MainWindowActiveDefaultBorderBrush}" BorderThickness="1" IsHitTestVisible="True">
<DockPanel>
<DockPanel x:Name="TitleBar" DockPanel.Dock="Top" VerticalAlignment="Top" WindowChrome.IsHitTestVisibleInChrome="False" Height="32">
<Button Style="{DynamicResource MainWindowButtonStyle}" DockPanel.Dock="Right" Content="{me:Image {StaticResource VectorCloseWindow}, 12, 12}" x:Name="CloseButton" Command="{x:Static cmd:SystemCommands.CloseWindowCommand}"/>
Copy link
Member

Choose a reason for hiding this comment

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

Any reasons for these to use dynamic resource binding? Dynamic resources are more expensive as they can't be cached and affect dramatically the performance. As much as possible, static resources should be used.

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 was going with static resources first, but then thought about the possibility of changing themes at runtime later on (as in VS). Right now you'd have to restart the editor.
I can change it to static resources so the change to dynamic resources can be made when it's required. I'll probably have to reorder some definitions then to appear in front of where they're used.

Copy link
Member

Choose a reason for hiding this comment

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

Yes please assume live theme switching won't be supported anytime soon (seems too heavy because of dynamic resources) and switch back to StaticResource.
We can always change it back if we decide to allow live theme switching, but it seems highly unlikely so far.

Copy link
Contributor Author

@RayKoopa RayKoopa Apr 26, 2020

Choose a reason for hiding this comment

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

Okay, I should also switch to static color resources in the AvalonDock theme then (which I'm updating right now in another branch to the most recent version) to make it consistent.

From what I know, colors are typically dynamic resources to also reflect system color changes (which became kinda rare, to be fair), but styles should indeed be static, unlike done above.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, restart for theme change is totally fine... Great to set this morning forward!

Copy link
Member

Choose a reason for hiding this comment

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

From what I know, colors are typically dynamic resources to also reflect system color changes (which became kinda rare, to be fair), but styles should indeed be static, unlike done above.

That's a good point. I'm wondering if we should also change them to be static. It probably needs some testing and performance comparison especially when working on a big project with lots of resources/assets and entities in a scene to see if it has any impact.

Let's keep the new resources in the theme static, and the color dynamic when they already were for now. We can review it later.

… reordering of styles / templates to be declared before usage.
@RayKoopa
Copy link
Contributor Author

I changed the newly introduced DynamicResources to StaticResources. It required reordering the main window button styles / templates.

@xen2
Copy link
Member

xen2 commented Apr 29, 2020

@Kryptos-FR with the latest changes (Dynamic to Static), it should be OK to merge?

@Kryptos-FR
Copy link
Member

@xen2 Yes. LGTM.

@xen2 xen2 merged commit cd751d6 into stride3d:master Apr 30, 2020
@RayKoopa RayKoopa deleted the ui_theme_update_and_fixes branch April 30, 2020 07:14
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.

7 participants