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

Allow width and height parameters in loadImage to support proper SVG scaling. #5779

Open
1 task done
annalogue-codes opened this issue Sep 1, 2022 · 8 comments
Open
1 task done

Comments

@annalogue-codes
Copy link

annalogue-codes commented Sep 1, 2022

Increasing Access

Allowing to use an SVG image as source for a canvas image leads to crisper images when a user scales the canvas up.

Most appropriate sub-area of p5.js?

  • Image

Feature enhancement details

The method p5.loadImage currently loads the given image in its inherent size. The underlying function drawImage actually allows drawing SVG files smoothly in any size, independent of resolution. It takes width and height arguments to do this. A quick way to make use of this would be to add these arguments to the signature of loadImage and pass them on to the call of drawImage, should they be given. If they are undefined then loadImage can still default to the inherent width and height.

Here is a proof of concept for implementing this:

p5.prototype.loadImage = function(
  path,
  successCallback,
  failureCallback,
  options
) {
  ...
    ...
      } else {
        // Non-GIF Section
        const img = new Image();

        img.onload = () => {
          pImg.width = pImg.canvas.width = options.width
            ? options.width
            : img.width;
          pImg.height = pImg.canvas.height = options.height
            ? options.height
            : img.height;

          // Draw the image into the backing canvas of the p5.Image
          pImg.drawingContext.drawImage(img, 0, 0, pImg.width, pImg.height);
          pImg.modified = true;
          if (typeof successCallback === 'function') {
            successCallback(pImg);
          }
          self._decrementPreload();
        };
        ...
      ...
    ...
  ...
  return pImg;
};

A possible use case would be a presentation with many graphics that should scale from small to large screens. Being able to use SVG files as templates for canvas bitmaps of appropriate size makes scaling presentations much easier, without the performance penalty of actually working with SVG elements.

@welcome
Copy link

welcome bot commented Sep 1, 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

Hi! It looks like in your PR #5781, when you set the image and canvas width/height to something other than the SVG's default width/height, it draws it to the canvas at a higher resolution. That's great to know! My question is mostly about how we should integrate that, which would need some questions answered first

  • Is it important to import SVGs in an otherwise bitmap context (as opposed to using p5js-svg, which makes everything render using SVGS?)
  • When importing an SVG, do people expect it to be vector as opposed to bitmap?
    • If it's vector, p5.Image forces you to pick a resolution up front. Maybe We could use what you discovered and make a different class that you can dynamically draw to whatever width/height you want?
    • If it's bitmap, then something like your PR's solution of adding width/height to loadImage could work. I'd maybe want to put both in an options object so that there's the potential to add other loading options in the future

@limzykenneth
Copy link
Member

As @davepagurek alluded to as well, If this feature is to be added as proposed in the PR, the signature of loadImage(path, successCallback, failureCallback, width, height) is not ideal for me. The success and failure callback will likely be more optional than width and height which arguable means it should comes before these callbacks.

The issue of image smoothing on the underlying canvas could also be a problem as well.

@annalogue-codes
Copy link
Author

annalogue-codes commented Sep 1, 2022

For performance reasons p5js-svg is not an option for many use cases that would still profit from scalable graphics.

My use case: I'm using p5.play v3 for an interactive presentation that will play on very large displays as well as mobile phones. With the relatively large number of sprites, using p5js-svg would be infeasible. But being able to use one SVG file per sprite (or rather one set of svg files per animation) would make it much easier to scale the presentation.

In this scenario I would like the SVG files to render as bitmaps. I'm not interested in changing the properties of the SVG afterwards and just would like to use the SVG files as resolution independent templates to create the animations in an appropriate size for the actual canvas size.

@davepagurek wrote:

  • When importing an SVG, do people expect it to be vector as opposed to bitmap?

    • If it's vector, p5.Image forces you to pick a resolution up front. Maybe We could use what you discovered and make a different class that you can dynamically draw to whatever width/height you want?
    • If it's bitmap, then something like your PR's solution of adding width/height to loadImage could work. I'd maybe want to put both in an options object so that there's the potential to add other loading options in the future

I think both these suggestions have their merits.

I agree that the signature of loadImage as in my PR is not a good one. It was just a lazy way to address the issue and showcase a solution without disrupting the other code. (I'm still a bit new to actually using Github collaboratively. Please bear with me when i don't follow the right steps in the right order, yet.)

@limzykenneth wrote:

The issue of image smoothing on the underlying canvas could also be a problem as well.

Could you describe this a bit more? I'm not sure I understand this correctly.

@davepagurek
Copy link
Contributor

It was just a lazy way to address the issue and showcase a solution without disrupting the other code.

This is a valid concern! If we want to add parameters before the success/failure callback, we'll have to overload the loadImage method to support multiple signatures so that we don't disrupt existing code. loadModel does this so one could reference how the source for that to see how to implement it.

(I'm still a bit new to actually using Github collaboratively. Please bear with me when i don't follow the right steps in the right order, yet.)

No problem, welcome 🙂

@davepagurek
Copy link
Contributor

davepagurek commented Sep 1, 2022

I think both these suggestions have their merits.

Whether or not it's worth it to add another class is a separate question, but if we do support SVGs as a separate object type from images, we could add a .toImage(width, height) to them to convert.

Here's a proof-of-concept sketch of what it could look like drawing an SVG at arbitrary resolutions and getting it as a p5.Image: https://editor.p5js.org/davepagurek/sketches/pfJuINokH Although a downside of this method is that I'm not sure how it would work with WebGL. Explicitly creating it with a width/height might be easier and less brittle there.

edit: maybe for WebGL mode, it can be given textureWidth and textureHeight properties that will be taken into account when used as a WebGL texture?

@trikaphundo
Copy link

Hello @davepagurek, in response to a previous comment of yours here

We might want it to mirror Processing's features: There's a Processing example being converted to js for the website (Make interactive live sketch for Shape examples processing-website#314) which was not able to use p5js-svg for performance reasons and they currently don't have a p5 solution.

You might be interested in my latest comment in that issue. I have ported those examples to P5js, with the aid of Paperjs library, and you might find interesting some of the reasons why I decided to use that library over others.

The bottom line is Paperjs renders things on a canvas, therefore you can make P5js and Paperjs to share the very same canvas, and draw things with one library or the other depending on whether you are working with SVG paths or not. Or use Paperjs to load and manipulate SVG files, and convert them to objects for later use with P5js, then ditching Paperjs stuff once you are done.

You can see here how to get started with Paperjs.

I hope this is, to some extent, helpful.

@davepagurek
Copy link
Contributor

Thanks @trikaphundo! It looks like paperjs loads SVGs into their own internal data structure so that they can manipulate them render them to the canvas. This would also be cool, and would mirror Processing's PShape more, although it would be a much larger change for p5.

I'm ok with a compromise solution where we only let you draw SVGs like they're images if we can make it easy to draw them without pixellation (either via drawing them at the right size automatically or manually specifying a width/height.)

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

4 participants