LingDong- / q5xjs

A small and fast alternative (experimental) implementation of p5.js
https://q5xjs.netlify.app/
The Unlicense
544 stars 25 forks source link

Shape quality #6

Closed mariosmaselli closed 3 years ago

mariosmaselli commented 3 years ago

Hey, thanks a lot for the lib, I'm testing it on a project and the js size decrease considerably. So far the only problem that I've found is the quality of the shapes, they seem a bit more blurred and not as sharp as with p5

Any idea what can be causing this? not sure if is related with pixelDensity, but I notice that this also doesn't work the same way as in p5

Thanks!

LingDong- commented 3 years ago

Hi! I believe it is indeed related with pixelDensity. p5.js implicitly sets pixelDensity(2) on retina displays. q5.js always starts with pixelDensity(1) by default, as I think it produces more consistent and predictable results when accessing the pixels[] array. However you can set the ratio to other values by explicitly calling e.g. pixelDensity(2);. The intrinsic pixel density of the device can be found with window.devicePixelRatio (Maybe I should wrap this one too like p5 does). Currently pixelDensity needs be called in setup() before drawing stuff, otherwise there might be some strange results with scaling. Please let me know if you encounter more issues, thanks a lot!

mariosmaselli commented 3 years ago

Is actually scaling down on q5, even if I add it on the setup https://codepen.io/mariosmaselli/pen/a3a9b31a6b7f7667153d3080b44e55a7

Is this the "normal" behaviour? should I re-scale afterwards Thanks for the help!

Screenshot 2020-10-03 at 23 03 40
LingDong- commented 3 years ago

Hi @mariosmaselli,

pixelDensity() needs to be called after createCanvas(), (since internally it really maps to a canvas/context upscale + canvas CSS downscale, if there's no canvas, there's nothing for it to scale :)

Looks like p5.js is fine with either order, I'll see if it makes sense for q5 to match this behavior. Thanks for the report!

Also the current release version on cdn.jsdelivr is tiny bit out of date, I just pushed a new release from the master branch that contains a related fix to background(). Should be working once jsdelivr relinks to my new version (it takes a while) and order of createCanvas and pixelDensity is flipped!

Screenshot below is what it looks like when I manually include master branch version. I'm on a retina display too.

image

Hope this helps, thanks!

mariosmaselli commented 3 years ago

Thanks a lot! Everything is working now.. Yeah I believe that I tried the other way around but the background was off, I updated q5 now and everything is good!