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

Refactoring tools #281

Merged
merged 7 commits into from
Jul 9, 2020
Merged

Refactoring tools #281

merged 7 commits into from
Jul 9, 2020

Conversation

Kinwailo
Copy link
Contributor

@Kinwailo Kinwailo commented Jul 7, 2020

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.

# Conflicts:
#	project.godot
#	src/Autoload/DrawingAlgos.gd
#	src/Autoload/Global.gd
#	src/Autoload/Import.gd
#	src/Canvas.gd
#	src/Main.tscn
@OverloadedOrama
Copy link
Member

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):
Copy link
Member

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.

Copy link
Contributor Author

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)

Copy link
Member

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.

@OverloadedOrama
Copy link
Member

OverloadedOrama commented Jul 7, 2020

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.

@Kinwailo
Copy link
Contributor Author

Kinwailo commented Jul 7, 2020

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.

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.

Okay I think everything works properly now, I did not find any other issue, so I'll go ahead and merge. Thank you!

@OverloadedOrama OverloadedOrama merged commit 4a668f7 into Orama-Interactive:master Jul 9, 2020
@OverloadedOrama
Copy link
Member

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.

@Kinwailo
Copy link
Contributor Author

Kinwailo commented Jul 9, 2020

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?

@Kinwailo Kinwailo deleted the dev branch July 9, 2020 19:41
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