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

fixes #175 - Text gets chopped when rotated >60 degrees. #191

Conversation

replaysMike
Copy link
Contributor

The glyph size is not rendered with enough space when rotated, this fixes the issue at all rotation angles

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

The font glyph isn't rendered with enough space to allow for text rotations. I modified the code that adjusts for that as part of the offset. Tested with a large range of fonts at varying rotations, and added appropriate tests for comparing expected output.

These tests will likely need to be modified when a fix is present for why rotated text gets offset shifted horizontally.

The glyph size is not rendered with enough space when rotated, this fixes the issue at all rotation angles
@CLAassistant
Copy link

CLAassistant commented Jan 7, 2022

CLA assistant check
All committers have signed the CLA.

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Jan 8, 2022

Thanks for this! I tried to push some fixes to your tests but Git LFS and GitHub don't work well together and won't let me do so. I'll comment with the required changes instead.

Copy link
Member

@JimBobSquarePants JimBobSquarePants left a comment

Choose a reason for hiding this comment

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

Can't really comment on the fix but have spotted some issues with the tests which are causing the offset your seeing. I've commented with the fixes.

@JimBobSquarePants JimBobSquarePants added area:drawing bug Something isn't working labels Jan 9, 2022
@JimBobSquarePants JimBobSquarePants added this to the 1.0.0-rc.1 milestone Jan 9, 2022
@JimBobSquarePants
Copy link
Member

@replaysMike Can you please do me a favor and update the following reference image? There's an invisible to the eye (0.0001%) diff being reported on our Windows CI build for this image and I can't get GitHub to let me make the change in your fork.
FontShapesAreRenderedCorrectly_Solid400x45_(255,255,255,255)_OpenSans-Regular.ttf-20-Sphi-(0,0).zip

Once that's updated and we have running tests I'll get this merged. Thanks!

@replaysMike
Copy link
Contributor Author

@replaysMike Can you please do me a favor and update the following reference image? There's an invisible to the eye (0.0001%) diff being reported on our Windows CI build for this image and I can't get GitHub to let me make the change in your fork. FontShapesAreRenderedCorrectly_Solid400x45_(255,255,255,255)_OpenSans-Regular.ttf-20-Sphi-(0,0).zip

Once that's updated and we have running tests I'll get this merged. Thanks!

will do thanks! I was just re-looking into this issue glad you already found the problem.

@codecov
Copy link

codecov bot commented Jan 13, 2022

Codecov Report

Merging #191 (3468b8c) into master (0ccea4d) will increase coverage by 0%.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #191   +/-   ##
=====================================
  Coverage      70%    70%           
=====================================
  Files          87     87           
  Lines        5116   5116           
  Branches     1062   1062           
=====================================
+ Hits         3593   3595    +2     
+ Misses       1308   1307    -1     
+ Partials      215    214    -1     
Flag Coverage Δ
unittests 70% <100%> (+<1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ssing/Processors/Text/DrawTextProcessor{TPixel}.cs 97% <100%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ccea4d...3468b8c. Read the comment docs.

@JimBobSquarePants
Copy link
Member

Let's get this merged! Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:drawing bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants