-
-
Notifications
You must be signed in to change notification settings - Fork 384
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
Refactoring tools #281
Refactoring tools #281
Conversation
# Conflicts: # project.godot # src/Autoload/DrawingAlgos.gd # src/Autoload/Global.gd # src/Autoload/Import.gd # src/Canvas.gd # src/Main.tscn
This'll take a while for me to review because of the amount of changes. Please keep in mind for the future that it's best to make multiple small Pull Requests for each new feature of bug fix you'd like to implement. See the CONTRIBUTING.md file for more details. |
var image := _get_draw_image() | ||
var color := image.get_pixelv(position) | ||
if _fill_with == 0 or _pattern == null: | ||
if tool_slot.color.is_equal_approx(color): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an issue here, probably related to this line. If I start the program, select a color (in my case, I selected the second color of the Default palettte), close the program, start it again and use the bucket fill twice, Pixelorama crashes. Oddly enough, if I select colors from the palette while the program is running (without closing it), the crash does not happen. It could also have to do with the color values not being saved properly in the config file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dunno how to change the PR. I added a processed pixel checking fix the bug.
image.lock()
var processed := BitMap.new()
processed.create(image.get_size())
var q = [position]
for n in q:
if processed.get_bit(n):
continue
var west : Vector2 = n
var east : Vector2 = n
while west.x >= project.x_min && image.get_pixelv(west).is_equal_approx(color):
west += Vector2.LEFT
while east.x < project.x_max && image.get_pixelv(east).is_equal_approx(color):
east += Vector2.RIGHT
for px in range(west.x + 1, east.x):
var p := Vector2(px, n.y)
_set_pixel(image, p.x, p.y, tool_slot.color)
processed.set_bit(p, true)
var north := p + Vector2.UP
var south := p + Vector2.DOWN
if north.y >= project.y_min && image.get_pixelv(north).is_equal_approx(color):
q.append(north)
if south.y < project.y_max && image.get_pixelv(south).is_equal_approx(color):
q.append(south)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can make changes to the PR by just committing and pushing to your dev branch. The PR will be synced automatically.
There is also an issue with the (Windows, at least) build generated from the CI workflow, if you download it from GitHub Action's artifacts. If I draw once using the Pencil tool, and then change the brush size, for some reason I'm no longer able to draw. I can't reproduce this issue from when running the project from Godot, or when I create a build from my own. I'm not sure why this is happening, it could be a CI or even a Godot bug. I originally thought that this was being caused by some Script Errors which I fixed in 7128a7f, but it didn't seem to be the case. (The fact that the Web export is failing is unrelated and not an issue) EDIT: The issue seem to occur only when using the pixel brush. |
I checked the config file, some config value saved in float. So the brush size changed to float and not working. I enforce convert to integer. But it seem a bug of CI/Godot on static typing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I think everything works properly now, I did not find any other issue, so I'll go ahead and merge. Thank you!
Update, I encountered a crash when selecting the bucket tool, while there are no Patterns saved. I think I have fixed it in 671536c, but feel free to take a look too. |
Maybe create a placeholder pattern with blank white image if there is no pattern imported, and use it as default pattern? |
Created scene and script for each tools, handle ui and logic on its own. A tools singleton manage tool and color slot, handle input event to drag-drop logic.
Some new feature:
Improved drawing speed.
Draw line by drag and drop.
Draw line with ctrl and pixel perfect on draw at 1:1 and 1:2. (#201)
Draw indicator outline. (merge the small rect together)
Show region and size at cursor label when using selection.
Remember tool and color slot and its options.
Fix bug:
Undo redo remove brush make the brush index incorrect.
Draw image brush on clipping bound incorrect.
Draw image brush with mirror does not mirror the image itself.