Skip to content

Add a function for specifying how to draw an image within set bounds without distorting its aspect ratio #5720

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

Closed
2 of 17 tasks
tetrogem opened this issue Jul 12, 2022 · 25 comments

Comments

@tetrogem
Copy link

tetrogem commented Jul 12, 2022

Increasing Access

This would allow p5 users to easily do things such as have unstretched images cover the entire background of a sketch, without needing to do the math required to find the scaling needed to have it cover the needed area, or only fit inside of it if wanted instead.

Most appropriate sub-area of p5.js?

  • Accessibility (Web Accessibility)
  • Build tools and processes
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Friendly error system
  • Image
  • IO (Input/Output)
  • Localization
  • Math
  • Unit Testing
  • Typography
  • Utilities
  • WebGL
  • Other (specify if possible)

Feature request details

This proposal is to add a new function similar to rectMode(), ellipseMode(), imageMode(), and textAlign(), which would be called imageFit() (working name). This function would change the width and height arguments of image() function to specify a bounding area for the image to be drawn within, instead of the width and height the image should be drawn with.

The imageFit() function would be able to take three arguments, (fit, xAlign, yAlign).
The fit argument would take the following new p5 constants:

  • STRETCH (default)
  • EXTEND
  • SHRINK

The xAlign argument would take the following p5 constants:

  • LEFT
  • CENTER (default)
  • RIGHT

The yAlign argument would take the following p5 constants:

  • TOP
  • CENTER (default)
  • RIGHT

The imageFit() function would be able to be called as:

  • imageFit(fit) -> xAlign: CENTER, yAlign: CENTER
  • imageFit(fit, xAlign, yAlign)

The imageFit() function would return the p5 instance, to allow for chaining (as do rect/ellipse/imageMode())

The following image will be used to demonstrate the different fitting types: (original file/aspect ratio)
IMAGE

The STRETCH fit would function identically to how images are drawn currently. It would draw the image on screen so that its width and height were equal to the arguments passed into the image() function.
STRETCH

The EXTEND fit would extend the shape so that it fills the entirety of the bounding area specified by the width and height arguments of the image() call. Parts of the image may also be drawn outside of this bounding area if the aspect ratio of the original image is not the same as the bounding area's.
EXTEND

The SHRINK fit would contain the shape within the bounding area specified in the image() function. This will result in some of the bounding area not being covered if the aspect ratio of the image is not equal to that of the bounding area.
SHRINK

The xAlign and yAlign arguments of the imageFit() function work identically to and use the same constants as the textAlign() function. These will specify how the image will be drawn within its bounding area after it has been fitted.
SHRINK_ALIGN
*** EDIT: THIS IMAGE IS MEANT TO SAY "EXTEND" IN THE TOP LEFT
SHRINK_EXTEND

(Optional) scale argument for the imageFit() function
The scale argument would allow a number to be passed in to scale the image by after it as been aligned and fitted to the bounding area:
SCALING

This would also then allow the imageFit() function to be called using any of the following arguments:

  • imageFit(fit) -> xAlign: CENTER, yAlign: CENTER, scale: 1
  • imageFit(fit, scale) -> xAlign: CENTER, yAlign: CENTER
  • imageFit(fit, xAlign, yAlign) -> scale: 1
  • imageFit(fit, xAlign, yAlign, scale)
@welcome
Copy link

welcome bot commented Jul 12, 2022

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, please make sure to fill out the inputs in the issue forms. Thank you!

@davepagurek
Copy link
Contributor

I think this sort of feature would be really useful! I've had to write manual versions of this a number of times for projects. Thanks for all the detailed diagrams too!

Some API questions and suggestions:

  • CSS has a similar API for background-size, and they use cover and contain instead of extend/shrink. We don't need to match this but do you think it would make things more approachable if we use the same terminology?
  • For p5 devs with more experience in this area than me, do you think this makes the most sense as an image mode, which persists, or an alternate image drawing function, so it just affects the current image being drawn? If one has to choose between drawing an image with fit parameters and drawing an image using e.g. the detailed image(img, dx, dy, dWidth, dHeight, sx, sy, [sWidth], [sHeight]) api, it might limit weird edge cases that we might run into.
  • If this ends up being an image drawing function, how would you feel about making the final parameter be an optional object for options instead of just a scale value? That way if there are other optional settings, you could set just the ones you need by specifying the keys you want without having to put undefined for the scale parameter. Some other properties we might consider adding:
    • clip: whether or not the parts outside the dedication size are visible when extending (could use the longer image(img, dx, dy, dWidth, dHeight, sx, sy, [sWidth], [sHeight]) internally to do the clipping)
    • tightBounds: much farther future reaching, but at work we have utility functions that find the bounding box of just the non transparent pixels in an image and then use that for fitting and alignment. It gets a lot of use at work so while this isn't a required feature at all, I think it could make a great follow-up in the future

@tetrogem
Copy link
Author

I do really like the idea to change the names of EXTEND and SHRINK to COVER and CONTAIN! I didn't know about the CSS API, but having them be consistent with that could help easily familiarize those who have done similar work in CSS with images.

I feel that this function would definitely work better as a one-off drawing function rather than one that persists like imageMode(), since most of the time you wouldn't want this effect to happen on multiple images in succession. The extended image() function image(img, dx, dy, dWidth, dHeight, sx, sy, [sWidth], [sHeight]) actually seems really similar in concept to the "bounding area" of fitting parameters to me (I had forgotten this extended form existed before you mentioned it), so I feel like the best option here might even be to rework the idea behind that to use bounding areas/fitting instead?

For example, image() would be redefined as being able to take the following arguments:

  • image(img, bx, by, [bWidth], [bHeight], [fit], [xAlign], [yAlign]) -> Sets the "bounding area" for where the image is drawn on the canvas, with optional arguments at the end for how to handle different aspect ratios. bWidth and bHeight must be supplied to change fit or alignment inside of bounding box. If bWidth and bHeight are not defined, they default to the size of the img, resulting in the same functionality as the current image(img, x, y). If bWidth and bHeight are defined but fit isn't, fit defaults to STRETCH, giving the same functionality as the current image(img, x, y, width, height).
  • image(img, dx, dy, dWidth, dHeight, bx, by, [bWidth], [bHeight], [fit], [xAlign], [yAlign]) -> Adds an extra 4 arguments between img and the bounding area specifying a region of the original image to be drawn (instead of the full image). This should give the same functionality as the current extended image function if only the arguments up to bHeight are defined, in order to be backwards compatible with the old image() implementation. Otherwise, this would function identically to the first set of arguments for image(), where only the bounding area is defined, except it would be as though the cropped area specified in the d arguments was a standalone image passed into the first version of the function, allowing for it to be covered/contained/stretched within the bounding area based on the fit parameters.

These changes should allow for all code currently using image() to still function identically, while providing the new fitting functionality as well in a way that reduces function clutter and allows for it to only affect a single image call instead of persisting. Some examples of the function being used in a program could look as follows:

  • image(img, 0, 0);
  • image(img, 0, 0, COVER);
  • image(img, 50, 100, 200, 400, CONTAIN, CENTER, TOP);
  • image(img, 50, 0, 40, 20);
  • image(img, 50, 0, 40, 20, 50, 50, 50, 50, COVER, LEFT, BOTTOM);

This does come with the problem still of maybe being too many arguments for a p5 function, but because most of them are optional, I feel that it would be alright since new programmers could use the current image() functionality still by omitting the fitting parameters.

@davepagurek
Copy link
Contributor

These changes should allow for all code currently using image() to still function identically, while providing the new fitting functionality as well in a way that reduces function clutter and allows for it to only affect a single image call instead of persisting.

This is great, I like this design a lot!

This does come with the problem still of maybe being too many arguments for a p5 function, but because most of them are optional, I feel that it would be alright since new programmers could use the current image() functionality still by omitting the fitting parameters.

Starting from `image(img, bx, by, bWidth], bHeight) and then incrementally adding fit parameters feels right, and like a small enough jump. My only worry would be that seeing all the options in the docs would be overwhelming, but like you showed above, it's really just two options, with optional parameters. So maybe leaning on having good examples is the way to go.

One other alternative could be using image(img, bx, by, [bWidth], [bHeight], [fitOptions]), where fitOptions is an object that looks like this (in Typescript syntax):

{
  fit: 'cover' | 'contain'
  xAlign?: 'left' | 'center' | 'right'
  yAlign?: 'top' | 'center' | 'bottom'
}

The benefits I see of this are:

  • it doesn't overwhelm the parameter list as much
  • it would let you specify fit + yAlign and use the default xAlign (using a standard parameter list, it would look like 'contain', undefined, 'top' if you wanted to do that)
  • Maybe it's easier to read at a call site because you'd see the keys next to the values in the object so you know what each thing is specifying?

The downside is that this would be a relatively new pattern in p5, so it would be a question of whether it's worth trading the above for a longer parameter list but a bit more API consistency. I could see that potentially being more important, so let me know if you have thoughts on that!

@tetrogem
Copy link
Author

One other alternative could be using image(img, bx, by, [bWidth], [bHeight], [fitOptions]), where fitOptions is an object
I can see this being a good method for making the function have less clutter, so this is definitely an option if that is deemed to be too big of an issue. I do also like the added benefit of then allowing for only the yAlign / xAlign to be defined without the other. Although, it is not too much harder to also just type the CENTER constant in-place for the xAlign parameter in the current version to then be able to specify a yAlign, and turning this into an object also has the added problem of allowing alignments to be passed in without a fit, which would be redundant and not do anything since the default fit is STRETCH, which would make it fit perfectly in the bounding area allotted already, and this could be confusing to new programmers.

Having the arguments be optional after the fit argument can help by showing which arguments are required for others to function naturally, as can be seen in image(). The fit parameter has no use unless a width and height are also passed, which must always be required currently since the fit argument is located after width and height. The same goes with the alignment parameters currently as they are after the fit parameter, which means a fit must currently be specified in order for alignments to also be specified. I could see these as possible benefits for the current arguments system, and this would be the one I would lean towards being implemented more because it does also match with the majority of the rest of p5 functions in syntax.

The downside is that this would be a relatively new pattern in p5, so it would be a question of whether it's worth trading the above for a longer parameter list but a bit more API consistency.

An options argument isn't actual unattested in p5 currently, as seen in the textBounds method for p5.Font objects, so it is definitely possible for this to be the way it gets implemented.

@davepagurek
Copy link
Contributor

That makes sense! I'll wait for some p5 image people to see if they've got any opinions but I'd feel good about going down your preferred route.

@stalgiag I think this is your area, how do you feel about this proposal?

@stalgiag
Copy link
Contributor

Hi @tetrogem thank you this detailed and considered proposal! Also @davepagurek thanks for jumping in and generating a valuable conversation. I really enjoyed reading the back and forth. While I am a steward of the Image section, this is more out of my relationship to GIFs and image decoding/encoding. Much of the design of image() predates my involvement. This just means that my opinion comes from general interest rather than deep familiarity with the reasoning behind the original design choices.

The concerns that are forefront in my mind have already been identified here: making sure old sketches don't break with any change to image() as it is a very core function and also making sure that the signature doesn't become too clunky.

Just stepping back a bit, I would love to think through examples where one doesn't want to distort the aspect ratio of an image but approaches like the following are insufficient:

I don't need to resize the image -

image(myImage, x, y)

I need to resize the image -

image(myImage, x, y, myImage.width * 0.6, myImage.height * 0.6)

I need to resize the image but I need it to be centered relative to original size -

imageMode(CENTER);
image(myImage, x + myImage.width / 2, y + myImage.height / 2, myImage.width * 0.6, myImage.height * 0.6);

I know some clumsiness is creeping in by that last example but it is possibly more legible than filling out the entire signature and adding an options object at the end. But it is very likely that I am missing some goals that would become much easier with the new proposal. Any ideas?

@davepagurek
Copy link
Contributor

My main use case is that I have an image that I want to be centered and resized to fit inside a box. Say I've got an image of a bird cage (the box) and an image of a bird (the image) and I want to always draw the bird centered inside the cage.

The logic I'd need to write to accomplish this gets pretty complicated when you consider the different combinations of aspect ratios:

const birdRatio = bird.width / bird.height
const cageRatio = cage.width / cage.height
let birdScale
if (birdRatio < cageRatio) {
  birdScale = cage.height / bird.height
} else {
  birdScale = cage.width / bird.width
}
const offsetX = (cage.width - birdScale*bird.width) / 2
const offsetY = (cage.height - birdScale*bird.height) / 2
image(cage, 0, 0)
image(bird, offsetX, offsetY, birdScale * bird.width, birdScale * bird.height)

Another scenario is if I have a box that I want to completely fill with an image. Maybe I'm making thumbnails for an image gallery and I want each thumbnail to be the same size, completely filled edge to edge with the image. I'd have to write similar code but flip the contents of the if and else branches.

With the new proposal, the bird and cage example would be:

image(cage, 0, 0)
image(bird, 0, 0, cage.width, cage.height, CONTAIN)

and making a thumbnail for an image gallery would be:

image(myImage, 0, 0, thumbnailWidth, thumbnailHeight, COVER)

@tetrogem
Copy link
Author

Another example of using COVER I use very often would be to cover the entire background of a sketch with an image, and allowing it update live based on both the size of the canvas (if it changes) and making sure it would work with any image passed into it. Currently, the code to do this would look like:
image(img, width / 2, height / 2, img.width > img.height ? height / img.height * img.width : width, img.width > img.height ? height : width / img.width * img.height);
An example of this code being used in a sketch can be found here: https://editor.p5js.org/TetroGem/sketches/3uLwLCzx-

@stalgiag
Copy link
Contributor

Thanks!

I can see the value. I would personally opt to use DOM elements in the image gallery and background examples but I see the value for others.

As for the design, wouldn't the simplest possible change that doesn't break current functionality be:

image(myImage, 0, 0, width, height, dx, dy, dWidth, dHeight, sx, sy, sWidth, sHeight, fit, xAlign, yAlign);

or something similar with an options object replacing fit, xAlign, yAlign.

@davepagurek
Copy link
Contributor

I think so! Tell me if I'm not on the same page but that's also how I'm reading @tetrogem's second proposal -- just those extra parameters, but with the sx, sy, etc renamed to be bx, by, etc. In any case I think that change feels minimal but quite useful :)

@davepagurek
Copy link
Contributor

Oh, my apologies, it is slightly different. Here's everything in one place so you can compare side-by-side:

// Original API
image(img, dx, dy, dWidth, dHeight, sx, sy, [sWidth], [sHeight])

// @TetroGem's proposal
image(img, dx, dy, dWidth, dHeight, bx, by, [bWidth], [bHeight], [fit], [xAlign], [yAlign])

// @stalgiag's proposal
image(img, dx, dy, dWidth, dHeight, bx, by, [sWidth], [sHeight], [fit], [xAlign], [yAlign])

The difference between the last two being, the destination/box size should be equivalent, and the sWidth/sHeight parameters are preserved to let you fit a portion of the source image into that destination box. This last one seems good to me!

@tetrogem
Copy link
Author

I don't know how clear this was in my original comment as I tried to keep it as brief as possible, but the API using image(img, dx, dy, dWidth, dHeight, bx, by, [bWidth], [bHeight], [fit], [xAlign], [yAlign]) as arguments should function identically to the current image(img, dx, dy, dWidth, dHeight, sx, sy, [sWidth], [sHeight]) API, just rephrased to have it be consistent with the idea of a "bounding area" as giving by the updated fitting API for image().

The d arguments represent the region of the original image to be drawn, and the b arguments represent the region on the canvas for the image to be drawn at, which should be the way it currently works if I'm not mistaken. It's just rephrasing it as being the shorter function's b arguments (from image(img, bx, by, [bWidth], [bHeight], [fit], [xAlign], [yAlign])), could help make this longer function be more easily understood by new programmers, since the shorter function could be reanalyzed as having a default set for the d arguments of image(img, 0, 0, img.width, img.height, ...), with the longer function just being a way for them to specify a specific crop instead of using the full image.

@davepagurek
Copy link
Contributor

The d arguments represent the region of the original image to be drawn

I think the source of the confusion might just be this. In the docs for image, the d ones are for the destination, and t he s ones are for the original image, e.g.:

dxNumber: the x-coordinate of the destination rectangle in which to draw the source image

@tetrogem
Copy link
Author

tetrogem commented Jul 13, 2022

Ah, I see I got the two mixed up. In that case, having the extended arguments list be:
image(img, bx, by, bWidth, bHeight, sx, sy, [sWidth], [sHeight], [fit], [xAlign], [yAlign])
should maintain backwards compatibility!

@manpreeeeeet
Copy link
Contributor

Hi, I would like to work on this.

@davepagurek
Copy link
Contributor

Hi @Manpreet-Singh001, I've assigned this to you 🙂

@manpreeeeeet
Copy link
Contributor

manpreeeeeet commented Aug 21, 2022

When the imageMode is set to CORNER the meaning of

fit: EXTEND/COVER
xAlign: LEFT
yAlign: BOTTOM

should still remain the same? example
image
this image should still produce the same result or am i missing something.

@tetrogem
Copy link
Author

In that example, the bottom of the image would be at the bottom of the bounding box, with the top of the image spilling out of the box instead.

@manpreeeeeet
Copy link
Contributor

manpreeeeeet commented Aug 21, 2022

In that example, the bottom of the image would be at the bottom of the bounding box, with the top of the image spilling out of the box instead.

fit: extend
xAlign: left
yAlign: bottom

image
so something like this even in CENTER MODE or this
image
since the mode is center the left should be center-left

@davepagurek
Copy link
Contributor

I think with xAlign: left and yAlign: bottom it would continue to look like your first image regardless of the imageMode specified. I think of imageMode as specifying the size/position of the purple box in your image, and then the xAlign/yAlign are responsible for positioning the image relative to the purple box.

@manpreeeeeet
Copy link
Contributor

CSS cover definition
Screenshot_20220821-113251_Chrome
but since the feature here wants to maintain the aspect ratio unlike css cover which will stretch, would it make more sense to call it EXTEND. Also if img is smaller than the canvas the image should be enlarged? since the example sent here https://editor.p5js.org/TetroGem/sketches/3uLwLCzx-
doesn't cover the entire thing if canvas is larger

@davepagurek
Copy link
Contributor

davepagurek commented Aug 22, 2022

CSS cover actually does maintain aspect ratio. I think by "stretch" they mean "scale it up past its original size."

I believe that example is just a demonstration and doesn't cover the case where the input image is smaller, but the ideal implementation would. I think instead of checking if the width if the source is greater than the destination, it needs to be checking if the aspect ratio of the source is greater than the aspect ratio of the destination, and resize however much is necessary to fit or cover the destination.

@davepagurek
Copy link
Contributor

@all-contributors please add @tetrogem for idea

@allcontributors
Copy link
Contributor

@davepagurek

I've put up a pull request to add @tetrogem! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants