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

Further speed up of bucket fill #672

Conversation

MatteoPiovanelli-Laser
Copy link
Contributor

By cutting down on some tests that were being repeated after a point had already been found to be one to be colored, I made the flood fill a bit faster again.

managing has_selection condition differently
This allowed shortcircuiting away some tests to speed up common code
paths.
Removed unused functions. Made conditions for pattern-fills clearer.
Copy link
Member

@OverloadedOrama OverloadedOrama left a comment

Choose a reason for hiding this comment

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

I tried it and it seems to work great! I was under the impression that the only way the bucket fill would speed up like this would be using GDNative, and I am glad to be proven wrong. Thank you!

@OverloadedOrama OverloadedOrama merged commit 1a19c3a into Orama-Interactive:master Apr 21, 2022
@Variable-ind
Copy link
Contributor

Variable-ind commented May 2, 2022

@MatteoPiovanelli-Laser I found a bug, pixels enclosed as shown here don't get effected by flood fill even if you click on top of it
the bug only occurs for the two cases below

bucket.mp4

@MatteoPiovanelli-Laser
Copy link
Contributor Author

I'm on the wrong machine to test this right now. Does the same issue happen regardless of whether you click to the right or to the left of the affected pixel? Looks to me I might have done a +2 where I needed to do a +1 or something like that where I'm computing where segments start. I'll see if I can check this later tonight.

@Variable-ind
Copy link
Contributor

yes, also i narrowed it down to this pull here: #681

@Variable-ind
Copy link
Contributor

i have narrowed it down further to line 305 of bucket.gd
is it safe to do if segment.right_position >= segment.left_position:
instead of if segment.right_position > segment.left_position:
cause that removes the bug

@MatteoPiovanelli-Laser
Copy link
Contributor Author

Yes, that actually makes a lot of sense. The code from my PR causes only segments at least two pixels long to be colored

@MatteoPiovanelli-Laser MatteoPiovanelli-Laser deleted the remove_can_get_drawn branch May 2, 2022 17:48
MatteoPiovanelli-Laser added a commit to MatteoPiovanelli-Laser/Pixelorama that referenced this pull request May 2, 2022
@MatteoPiovanelli-Laser
Copy link
Contributor Author

@Variable-ind do you want to go ahead and do the PR with the fix or should I?

@Variable-ind
Copy link
Contributor

i think you should do it 😃
it's just a single line change right?
if segment.right_position = segment.left_position: to if segment.right_position >= segment.left_position:
in that case we could ask @OverloadedOrama to commit it internally (in that case the pull wont be necessary)

@MatteoPiovanelli-Laser
Copy link
Contributor Author

Yes, that's literally the change required to fix the bug you found

@OverloadedOrama
Copy link
Member

Fixed in 9e7dd12

OverloadedOrama added a commit that referenced this pull request May 20, 2022
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