api icon indicating copy to clipboard operation
api copied to clipboard

Image:putImage() with a position gives varying results

Open christopherwk210 opened this issue 5 years ago • 5 comments

The docs state that If position [the second argument] is a point, it will put the given image in the given position with regard to Image:putImage(). I would expect the Point given to be relative to the Sprite, (0, 0) being the top left. I've gotten different results with this, however.

On one sprite, it seemed that the position given would be relative to the top left of the image already existing in that cel. On a blank cel of the same sprite, it was relative to the top left of the Sprite which is what I would expect.

If I create a brand new sprite however, instead of moving the image, it actually puts the image in the very top left no matter what, and the point given actually clips the sprite.

To reproduce the clipping bug, create a new sprite of any size and draw anything. Then execute the attached Lua script and choose "Apply to new layer".

color-overlay.lua.zip

christopherwk210 avatar Nov 05 '18 15:11 christopherwk210

It will work as intended If you replace:

cel.image:putImage(copy, imageCoords)

With:

cel.image = copy
cel.position = imageCoords

I believe the clipping behaviour is due to there being no image in the newly created cel and the fact that putImage replaces pixels in an image and not the image itself - It looks like a not covered edge case? Or maybe It's just how this API works and this needs to be remembered by people who write scripts? In the latter case would be nice to add to docs.

thkwznk avatar Nov 06 '18 12:11 thkwznk

Thanks for the solution, worked as expected! I actually had no idea that the image and position properties were setters as well.

Yeah, if the API for putImage is actually working correctly here then the docs should be updated to reflect this behavior.

christopherwk210 avatar Nov 06 '18 14:11 christopherwk210

I'm not sure how this should behave, but it looks like the most expected behavior is that the coordinates should be in sprite coordinates (instead of the specific image coordinates).

I think we should have two kind of functions:

  1. One set of functions that use absolute coordinates (sprite canvas coordinates)
  2. Another set of functions that use the specific image coordinates (which is a more internal thing, it's confusing for the user because they don't know that cel images / layer boundaries are different depending on the cel content)

In this case we could have for:

  1. putAbsPixel/putAbsImage (absolute position in the sprite canvas)
  2. putRelPixel/putRelImage (relative to the cel position)

I'm not sure about the function names, or if we should have some kind of "mode" (so we could change the reference point of putPixel/putImage, relative to the sprite (absolute), or relative to the cel position).

Other possibility would be to have a app.putPixel or Sprite:putPixel (to put pixels in the active image with absolute sprite canvas coordinates), but I'm not sure.

Any idea will be welcome 🤗

dacap avatar Nov 06 '18 23:11 dacap

I think the only thing that requires fixing is the odd behavior of cropping that @christopherwk210 described.

Otherwise, I believe all is working properly and only some visual aid for what exactly sprite, image, frame, cel, layer are and how they relate would be amazingly useful.

thkwznk avatar Nov 07 '18 07:11 thkwznk

I definitely am not against the ideas you've had @dacap! Absolute coordinates is what I assumed to be the default, but being able to use both positioning methods would be a plus I think.

I almost wonder if making it an argument rather than separate functions would be less work for you, i.e. putImage(copy, imageCoords, abs) where abs is a boolean? I guess it's arbitrary how you handle it, but I like the idea of being able to use both methods.

A visual aid would also be cool in the docs to go along with this as well. If there's anything I can do to help, let me know!

christopherwk210 avatar Nov 07 '18 16:11 christopherwk210