Skip to content

Render glyphs with nonzero IntersectionRule - #36

Merged
tocsoft merged 5 commits into
masterfrom
sw/nonzero-complexpolygons
Apr 18, 2020
Merged

tocsoft merged 5 commits into
masterfrom
sw/nonzero-complexpolygons

Conversation

@tocsoft

@tocsoft tocsoft commented Apr 11, 2020

Copy link
Copy Markdown
Member

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

Ensure we render glyphs using nonzero rule to prevent holes when using certain fonts.

Also fixed a side issue when starting trying to render a glyph completely to the left/above the image.

Fixes #35

int startY = operation.Location.Y;
int startX = operation.Location.X;
int offsetSpan = 0;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This fixes the issue where it fails when you try and render a glyph off image.


[Theory]
[WithSolidFilledImages(200, 200, "White", PixelTypes.Rgba32)]
public void OpenSansJWithNoneZeroShouldntExtendPastGlyphe<TPixel>(TestImageProvider<TPixel> provider)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is tracking a regression the switch to nonzero caused

@tocsoft
tocsoft requested a review from a team April 11, 2020 22:42
Span<PointF> subBuffer = buffer.Slice(totalAdded);
int added = this.paths[i].FindIntersections(start, end, subBuffer, intersectionRule);
totalAdded += added;
lock (this.paths)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do you need the lock here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

because I expect this methods to be call in parallel and I only want the flattening and instantiation of the InternalPaths are relatively expensive operations.

I did however miss the 2nd null check to make sure I didn't actually run the code a 2nd time.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe this should be moved to a separate method?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

FindIntersections() is a method on the IPath interface and only ComplexPolygons have this sort of initialization as its trying to efficiently composite the simplifed forms off all its child paths (which themselves can be ComplexPolygons)... not sure its worth adding all the complexity at this stage that change can probably be added when we review the API shapes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I tend to agree. Get it working now so we have a reference to optimize and refactor against.

@JimBobSquarePants JimBobSquarePants left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's definitely look to remove that lock. But good enough for now.

@tocsoft
tocsoft merged commit bbb764e into master Apr 18, 2020
@tocsoft
tocsoft deleted the sw/nonzero-complexpolygons branch April 18, 2020 16:00
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.

Font renders with holes

3 participants