Skip to content

Change while logic#1

Merged
JimBobSquarePants merged 2 commits into
SixLabors:masterfrom
stefannikolei:sn/countours
Jan 22, 2025
Merged

Change while logic#1
JimBobSquarePants merged 2 commits into
SixLabors:masterfrom
stefannikolei:sn/countours

Conversation

@stefannikolei
Copy link
Copy Markdown
Contributor

  • need to be while(true) otherwise this leads to a early return

* need to be while(true) otherwise this leads to a early return
@JimBobSquarePants
Copy link
Copy Markdown
Member

Amazing, I was looking everywhere else for the issue.

There are 2 hard problems in computer science: cache invalidation, naming things, and off-by-1 errors.

-- Leon Bambrick

@JimBobSquarePants JimBobSquarePants merged commit e8b1416 into SixLabors:master Jan 22, 2025
@JimBobSquarePants
Copy link
Copy Markdown
Member

@stefannikolei Looks like I've still got some debugging to do. Despite everything else being correct, there's a hole registered that shouldn't exist.

image

I've done a lot of cleanup in the repo code which should make things easier to debug if you fancy a pop.

@stefannikolei
Copy link
Copy Markdown
Contributor Author

@JimBobSquarePants The js also has that hole
image
But it does some other things after connecting the edges

@stefannikolei
Copy link
Copy Markdown
Contributor Author

stefannikolei commented Jan 24, 2025

@JimBobSquarePants I just looked deeper into the test code. I realised that the HoleCount will never be greater than 0. It is never set in ConvertToPolygon

Is the hole count interesting for the consumer at the end, or is it just an internal thing? Probably only the coordinates are from interest for the consumer? Or?

@JimBobSquarePants
Copy link
Copy Markdown
Member

JimBobSquarePants commented Jan 24, 2025

@JimBobSquarePants I just looked deeper into the test code. I realised that the HoleCount will never be greater than 0. It is never set in ConvertToPolygon

Is the hole count interesting for the consumer at the end, or is it just an internal thing? Probably only the coordinates are from interest for the consumer? Or?

I just this moment realised that!!

image

It should be something that is captured should the user be using different filling rules. NonZero expects CW for outer contours and CCW for inner holes. This seems like a massive oversite in the testing to me.

@stefannikolei stefannikolei deleted the sn/countours branch July 12, 2025 18:36
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.

2 participants