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

Change the handling of out-of-bounds canvas #255

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

EpicDima
Copy link
Contributor

@EpicDima EpicDima commented Nov 18, 2023

Example from #254

The first picture Screenshot 2023-11-18 at 18 02 05
The second picture Screenshot 2023-11-18 at 18 02 28

It seems to be the partial solution for point 5 in #153

Update:
Now this is a simple logic that does not cause crashes, while it allows you to partially draw objects that are partially shifted beyond the border. In the first commit, the canvas expanded automatically, but since this is done in the get method, offset modifier expanded the boundaries, although it should not be as a modifier.

if (widthDiff == 1) {
rows.forEach { it.add(newBlankPixel) }
} else {
rows.forEach { it.addAll(createBlankRow(widthDiff)) }
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 think this is not an ideal solution, because several lists are created, but on the other hand it should not be a hot execution path.

@JakeWharton
Copy link
Owner

I would like to write tests for this behavior before committing

@EpicDima EpicDima force-pushed the epicdima/change_work_beyond_canvas_borders branch from 7a2f4c4 to 8dd3870 Compare December 2, 2023 12:01
@EpicDima EpicDima force-pushed the epicdima/change_work_beyond_canvas_borders branch from 8dd3870 to 2e592ea Compare January 8, 2024 15:04
@EpicDima EpicDima force-pushed the epicdima/change_work_beyond_canvas_borders branch from 2e592ea to 6d1073b Compare January 19, 2024 21:24
@JakeWharton
Copy link
Owner

I haven't forgotten about all these PRs. I'm just busy, but they hover near the top of my "TODO" list and "Feels-bad-about-not-yet-merging" list.

@EpicDima EpicDima force-pushed the epicdima/change_work_beyond_canvas_borders branch from 6d1073b to 9df5a80 Compare April 20, 2024 16:13
@EpicDima EpicDima force-pushed the epicdima/change_work_beyond_canvas_borders branch from 9df5a80 to 94800f1 Compare May 16, 2024 08:49
@EpicDima EpicDima force-pushed the epicdima/change_work_beyond_canvas_borders branch from 94800f1 to 3be531a Compare May 29, 2024 08:39
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

2 participants