search

adventuregamestudio / ags

AGS editor and engine source code
Other
708 stars 159 forks source link

Editor: Fix room editor flood fill #2490

Closed KeyserSoSay0 closed 3 months ago

KeyserSoSay0 commented 3 months ago

-It looks like images can have a different _paddedWidth that needs to be taken into account when calculating where in the byte array we are. The flood fill algorithm was using the normal width instead of the padded width, so it would basically always flood the entire screen since the pixels would be off.

-Also, added a minor optimization. In our inner loop were were checking image[i] == initial && image[i] != replacement. The only time you could ever pass the initial check but fail the replacement check is if initial == replacement. In that case you're just doing if (a == b && a != b), so we can just skip the entire function for cases where the initial and replacement values are the same.

KeyserSoSay0 commented 3 months ago

I think I didn't completely mess up the pull request this time? Fingers crossed for slightly figuring out git.

ericoporto commented 3 months ago

Can you give some steps to reproduce the bug you are reporting? I think I need to have some size of room background to trigger it. Also can't tell if this happens in master.

The commit message title is missing PREFIX: and is too long. It would be better as something like Editor: fix room editor flood fill and then just quickly describe what is fixing in the next paragraph.

KeyserSoSay0 commented 3 months ago

Based on the code, it should be any image with a width that isn't divisible by 4. If that doesn't work on the first try though, I can get the exact image size I'm using when I get back home tonight.

I bet it doesn't happen in master because I don't remember having any issues with flood fill until recently. So I bet that the paddedWidth code is new and just missed a spot.

You should be able to reproduce by basically trying to flood fill in any shape. It will flood fill everything without this fix.

KeyserSoSay0 commented 3 months ago

It looks like my image is 358x205. Here's the actual background I'm using: starshipbridgeSm

Here's the walkbehinds I was editing when I noticed it (but it's probably easier to repro by just making your own lines) walkbehinds

ivan-mogilko commented 3 months ago

I confirm the error, and that the fix works. The code changes make perfect sense to me too.

How this mistake occurs may be seen on the following screenshot: a fill operation gets "cut" at a diagonal line: ags4-floodfill-error