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

sw_engine shape: improvement of the rectangle fast tracking algorithm #126

Merged
merged 1 commit into from
Nov 2, 2020

Conversation

mgrudzinska
Copy link
Collaborator

@mgrudzinska mgrudzinska commented Oct 29, 2020

Before (wrong):
stressErr

After (correct):
stressCor

Copy link
Member

@JSUYA JSUYA left a comment

Choose a reason for hiding this comment

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

Nice catch. please check my comment

auto min1 = pt1->y < pt3->y ? pt1 : pt3;
auto min2 = pt2->y < pt4->y ? pt2 : pt4;
if (min1->y != min2->y) return false;
if (*pt1 == *pt4 && *pt2 == *pt3) return false;
Copy link
Member

Choose a reason for hiding this comment

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

Is this condition necessary?
I think, In tvgSwShape's updateBBox, width and height are already being checked.
I think fastTrack only needs to check if it is rectangle.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this condition prevents from filling the whole bbox when the rect has a zero width.
Since the pictures in the Stress example were so small, the zero width rect occured sometimes and that was the reason why we saw 'the flashes'.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I misspelled 'zero width rect' when I meant 'zero width parallelogram' - there are, of course, parallelograms on the pictures in the description.
I also found an issue in my previous solution, so I decided to completely change the algorithm.
Please have a look now.

The algorithm erroneously treated some shapes (like isosceles trapezoids
and specifically arranged zero width parallelograms) as rectangles,
which causes the whole bbox to be filled.
Copy link
Member

@JSUYA JSUYA left a comment

Choose a reason for hiding this comment

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

Hi I am on vacation(~ November 13). So I couldn't actually run this change.
I think this code is fine.
please check again @hermet

@hermet hermet self-assigned this Nov 2, 2020
@hermet hermet self-requested a review November 2, 2020 08:59
@hermet hermet assigned mgrudzinska and unassigned hermet Nov 2, 2020
@hermet hermet merged commit a96037c into thorvg:master Nov 2, 2020
@mgrudzinska mgrudzinska deleted the mgrudzinska/Stress_fixed branch November 26, 2021 13:50
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.

None yet

3 participants