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

Blur the edges of UI background shapes #960

Merged
merged 1 commit into from Mar 27, 2019
Merged

Blur the edges of UI background shapes #960

merged 1 commit into from Mar 27, 2019

Conversation

MortimerGoro
Copy link
Contributor

@MortimerGoro MortimerGoro commented Feb 19, 2019

See #959 (tries to improve the first point on navigation bar and rounded corner dialogs)

This is a simple approach to add some blur, just adding a semitransparent stroke between the shape and the transparent border.

We can set a different size in <dimen name="blur_radius">2dp</dimen>

We could also set different radius values per platform (e.g this is more needed on Wave than on Oculus right now)

Anyone else that could test this on Vive Focus? (@philip-lamb @thenadj ?)

@ghost ghost assigned MortimerGoro Feb 19, 2019
@ghost ghost added the in progress label Feb 19, 2019
@cvan cvan self-requested a review February 20, 2019 00:48
@thenadj
Copy link

thenadj commented Feb 20, 2019

@MortimerGoro I don't have a Vive Focus to test this on.

@bluemarvin
Copy link
Contributor

I've looked at this and played with the border values. I'm not sure it makes enough of a difference. @cvan have you tried this PR? We should probably make a decision on this.

Copy link
Contributor

@cvan cvan left a comment

Choose a reason for hiding this comment

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

👍 I say let's ship this. However, I will admit that I myself cannot discern any differences in the UI visual fidelity in this branch vs. that of master and 1.1.2 and 78b00f5.

I certainly don't see any visual degradation or artifacting. and, given the other improvements here, such as the colour additions and stroke changes, I'd say this is good to ship.


P.S. for reference, here are some screenshots of the UI between the three versions I tested:

1.1.2 (4049807)

2019-03-13_01:24:54
2019-03-13_01:24:40
2019-03-13_01:24:28
2019-03-13_01:23:57
2019-03-13_01:23:48
2019-03-13_01:23:37
2019-03-13_01:23:27

master (78b00f5)

2019-03-13_01:18:39
2019-03-13_01:18:27
2019-03-13_01:18:15
2019-03-13_01:18:01
2019-03-13_01:17:54
2019-03-13_01:17:45
2019-03-13_01:17:36
2019-03-13_01:17:24
2019-03-13_01:17:18
2019-03-13_01:17:02
2019-03-13_01:16:55
2019-03-13_01:16:32

this branch (#960)

2019-03-13_01:24:54
2019-03-13_01:24:40
2019-03-13_01:24:28
2019-03-13_01:23:57
2019-03-13_01:23:48
2019-03-13_01:23:37
2019-03-13_01:23:27

@bluemarvin
Copy link
Contributor

@cvan I think this patch should just added a blur around the URL tool bar and its buttons. I would guess any other layout changes are from other patches. Is that true @MortimerGoro?

@MortimerGoro
Copy link
Contributor Author

It also adds a blur on the background of dialogs and prompts.

The auth promp is a good test to check the difference: https://www.httpwatch.com/httpgallery/authentication/authenticatedimage/default.aspx

e.g.: I think the edges of the prompt have less judder/flickering

@philip-lamb philip-lamb added this to the v1.1.4 milestone Mar 18, 2019
@philip-lamb philip-lamb added the enhancement This issue is a new feature or request label Mar 18, 2019
Copy link
Contributor

@cvan cvan left a comment

Choose a reason for hiding this comment

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

👍 looks good to me. tested this on master, this branch, and this branch rebased against master.

everything looks good: the URL bar, the Basic Auth prompt, etc.

@MortimerGoro MortimerGoro merged commit 8b6649c into master Mar 27, 2019
@ghost ghost removed the in progress label Mar 27, 2019
@cvan cvan deleted the edge_blur branch March 29, 2019 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This issue is a new feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants