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

CueStyler: Fix outline rendering of TimedTextCue elements #9224

Open
wants to merge 1 commit into
base: winui3/release/1.5-stable
Choose a base branch
from

Conversation

softworkz
Copy link

The current method of simulating text outlines is wrong in the way how the visuals are offset in diagonal directions.

Also, the number of visuals for simulating the effect is insufficient for creating an acceptable appearance.

I will follow up with some comparisons.

@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Jan 11, 2024
@softworkz
Copy link
Author

Comparison 1: Large Outline Values

Current Implementation

image

This PR but with 8 Iterations

image

This PR but with 16 Iterations

image

This PR (using 32 Iterations)

image

@softworkz
Copy link
Author

Comparison 2: Smaller Outlines, Bold and Rounded Font

Current Implementation

image

This PR but with 8 Iterations

image

This PR but with 16 Iterations

image

This PR (using 32 Iterations)

image

@softworkz
Copy link
Author

softworkz commented Jan 11, 2024

Comparison 2b: Same as 2 but scaled to 200%

Current Implementation

image

This PR but with 8 Iterations

image

This PR but with 16 Iterations

image

This PR (using 32 Iterations)

image

@softworkz
Copy link
Author

Further improvement would be to:

  • Remove the alpha component from the outline color before setting it on the elements
  • Put all outline elements into a common container
  • Set the alpha value of the outline color as opacity on that common container
  • This will also allow to implement TimedTextStyle.OutlineRadius (which is currently ignored) by applying a blur effect to that common container

@bpulliam bpulliam removed the needs-triage Issue needs to be triaged by the area owners label Feb 23, 2024
@softworkz
Copy link
Author

@bpulliam - What does the removal of the "needs-triage" label mean? What is the outcome of your triage?

@MartyIX
Copy link
Contributor

MartyIX commented Apr 14, 2024

@bpulliam Could this PR be reviewed please?

@DmitriyKomin DmitriyKomin self-assigned this Apr 15, 2024
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.

4 participants