search

paperjs / paper.js

The Swiss Army Knife of Vector Graphics Scripting – Scriptographer ported to JavaScript and the browser, using HTML5 Canvas. Created by @lehni & @puckey
http://paperjs.org
Other
14.44k stars 1.22k forks source link

Shadow doesnt scale when zooming the view #831

Closed tobmaster closed 8 years ago

tobmaster commented 8 years ago

When I define a Symbol/item like this

new paper.Symbol(
      new paper.Shape.Circle({
        center: [0, 0],
        radius: 5,
        fillColor: 'lime',
        strokeColor: 'green',
        shadowColor: new Color(0, 0, 0),
        shadowBlur: 6,
        shadowOffset: new Point(3, 3)
      })
    );

I expected the shadow to resize as the circle does. Instead its size is steady and changes the distance to the circle when zooming out and is disappearing behind the circle when zooming in.

As I am new to paperjs it still might do something wrong but it appears as a bug.

lehni commented 8 years ago

This is a canvas weirdness and not necessarily a Paper.js bug. From the Mozilla website:

this value doesn't correspond to a number of pixels and is not affected by the current transformation matrix.

https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/shadowBlur

We could try and "fix" this in paper.js of course, but I'm not sure how many people are already expecting the current behavior... Or we could make it controllable through a item.shadowScaling property, similar to the already present item.strokeScaling.

sapics commented 8 years ago

:+1: item.shadowScaling Just my two cents, it would be useful, scaling with item scaling at item.shadowScaling = 1 (which is similar to strokeScaling = true), and scaling with view._matrix.scaling at item.shadowScaling = 2.

iconexperience commented 8 years ago

Even if this is the behaviour specified by canvas, it is really weird. When zooming I would expect the whole view to appear just as if I looked through a magnifying glass, which means the shadow should change its extent and offset accoring to the zoom factor. I think PaperJS should add a way to fix this behaviour.

I have tried to simulate the expected behaviour. Here is the Sketch. (Update: I have added a second circle. The circle at the left shows the current behaviour, the one on the right has the corrected behaviour. Use Sketch's zooming at the top right to see the difference).

It is not perfect, but probably good enough. Here is a comparison between the original circle and zooming it with 4.76, then scaling it back to original size with an image editor:

image

So I agree, item.shadowScaling :+1:

tobmaster commented 8 years ago

Thanks for the nice tip and the answers. That tip should work for me now. Even when that can become slightly harder to maintain through the application lifecycle. The behavior is weird in canvas yes, a library should try to abstract that away or provide a way to control that behavior. So i think shadowScaling is the way to go looking for the backward compatibility.

iconexperience commented 8 years ago

I found this strange behavior that if I use browser zooming and reload a page, the shadow offsets are changed. Actually, it stays the same, but the proportions change.

Here is an example. At the left is the browser at 100%, at the right is the browser at 75%.

image

And here is the Sketch that I am using

When zooming in and out once the image is displayed, the proportions stay the same, until the script is run again.

Before I thought this would only affect scaling from inside paper.js., but this is quite a strange behaviour and I think it should be fixed.

@lehni Do you think there is a way to only respect browser zooming, but no other scaling for the shadows?

lehni commented 8 years ago

Which browser?

lehni commented 8 years ago

This looks like a Chrome bug. Safari doesn't behave this way. But Firefox does the same. Ah man.

iconexperience commented 8 years ago

Chrome, Firefox and InternetExplorer 11

iconexperience commented 8 years ago

Do we have access to the browser`s zoom factor from inside paper.js at all?

lehni commented 8 years ago

It looks like a real pain: http://stackoverflow.com/questions/1713771/how-to-detect-page-zoom-level-in-all-modern-browsers

iconexperience commented 8 years ago

I was just about to post the same link. This looks horrible.

iconexperience commented 8 years ago

Here is a little non-paper.js test case that uses canvas shadows. It seems that the problem does not occur here, so maybe we have a chance to fix without having to get the browser's zoom factor:

<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="UTF-8">
        <title>Test for Canvas Shadow Offset</title>
    </head>
    <body>
        <canvas id="myCanvas" width="512" height="512"></canvas>
        <script>
            var canvas = document.getElementById('myCanvas'),
                context = canvas.getContext('2d');

            context.rect(100, 384, 28, 30);
            context.fillStyle = 'orangered';
            context.fill();
            context.beginPath();
            context.rect(128, 128, 256, 256);
            context.fillStyle = 'mediumaquamarine';
            context.shadowColor = '#000';
            context.shadowBlur = 0;
            context.shadowOffsetX = 0;
            context.shadowOffsetY = 30;
            context.fill();
        </script>
    </body>
</html>

image

The height of the little orange rectangle should always have the height of the balck rectangle (which is the shadow).

lehni commented 8 years ago

Interesting. You can use the internal ProxyContext class to produce the sequence of drawing commands used by paper.js to draw the scene. This can then also be extracted into such a "paperless" file, and from there you can maybe find out at what causes the offsets:

view._context = new ProxyContext(view._context);
iconexperience commented 8 years ago

OK, I will look into this one.

tobmaster commented 8 years ago

Hey guys. I just want to note, my issue was about zooming with the paper.js view. You are now talking about Browser zooming I guess that is another thing or not?

iconexperience commented 8 years ago

@tobmaster Yes, but it's related, and I guess it's easiest to fix them at the same time. I hope you don't mind if we keep the together. We will certainly open a new issue if we decide that only one part will be fixed first.

iconexperience commented 8 years ago

OK, I have hooked the ProxyContext on the view's context, but the only difference that I can see is that a different area is cleared via clearRect() at the beginning. This should not make a difference.

Here is my sketch

And this is the output:

transform: undefined
ctx.clearRect(0, 0, 1727.9847412109375, 1527.9801025390625); // this is the only part that changes
ctx.save();
    ctx.save();
        ctx.globalAlpha = 1;
        ctx.save();
            ctx.globalAlpha = 1;
            ctx.beginPath();
            ctx.moveTo(100, 200);
            ctx.bezierCurveTo(100, 144.77152501692063, 144.77152501692063, 100, 200, 100);
            ctx.bezierCurveTo(255.22847498307937, 100, 300, 144.77152501692063, 300, 200);
            ctx.bezierCurveTo(300, 255.22847498307937, 255.22847498307937, 300, 200, 300);
            ctx.bezierCurveTo(144.77152501692063, 300, 100, 255.22847498307937, 100, 200);
            ctx.closePath();
            ctx.fillStyle = "rgb(218,165,32)";
            ctx.shadowColor = "rgb(0,0,0)";
            ctx.shadowBlur = 10;
            ctx.shadowOffsetX = 0;
            ctx.shadowOffsetY = 100;
            ctx.fill("nonzero");
            ctx.shadowBlur = 0;
        ctx.restore();
    ctx.restore();
ctx.restore();
lehni commented 8 years ago

And did you try this in an isolated HTML container along with the zooming, to see if the same occurs?

lehni commented 8 years ago

It doesn't happen here on Chrome: https://jsfiddle.net/n2xqmozz/

lehni commented 8 years ago

Perhaps it's got something to do with CSS styling of http://sketch.paperjs.org/

lehni commented 8 years ago

Ok, found it. The weird scaling happens because the zoom level changes window.devicePixelRatio on these browsers, and we're using its initial value to set up HiDPI support. These two issues are indeed linked...

https://github.com/paperjs/paper.js/blob/develop/src/view/CanvasView.js#L53

lehni commented 8 years ago

Here the HTML that replicates your problem:

<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<title>Zoom</title>
<script type="text/javascript">
document.addEventListener("DOMContentLoaded", function(event) {
    var canvas = document.getElementById('canvas');
    var ctx = canvas.getContext('2d');
    var deviceRatio = window.devicePixelRatio || 1;
    ctx.scale(deviceRatio, deviceRatio);
    ctx.clearRect(0, 0, 1000, 1000);
    ctx.save();
        ctx.beginPath();
        ctx.moveTo(100, 200);
        ctx.bezierCurveTo(100, 144.77152501692063, 144.77152501692063, 100, 200, 100);
        ctx.bezierCurveTo(255.22847498307937, 100, 300, 144.77152501692063, 300, 200);
        ctx.bezierCurveTo(300, 255.22847498307937, 255.22847498307937, 300, 200, 300);
        ctx.bezierCurveTo(144.77152501692063, 300, 100, 255.22847498307937, 100, 200);
        ctx.closePath();
        ctx.fillStyle = "rgb(218,165,32)";
        ctx.shadowColor = "rgb(0,0,0)";
        ctx.shadowBlur = 10;
        ctx.shadowOffsetX = 0;
        ctx.shadowOffsetY = 100;
        ctx.fill("nonzero");
        ctx.shadowColor = "rgba(0,0,0,0)";
    ctx.restore();
});
</script>
</head>
<body>
    <canvas id="canvas" width="1000" height="1000"></canvas>
</body>
</html>
iconexperience commented 8 years ago

Yes, the scaling happened before I could exchange the context with the ProxyContext. I worked around this by inserting these lines before initializing the paper project:

canvas.getContextOriginal = canvas.getContext;
canvas.getContext = function(a) {
    return new ProxyContext(canvas.getContextOriginal(a));
}

Anyway, the result is the same.

So this means that even without browser zooming the shadow offset will be different on HiDPI environments than on standard environments?

lehni commented 8 years ago

No, it just means that when fixing the weird shadow behavior we also need to take this initial scaling into account. It can all be solved together, I think. But it's not very high priority at my end...

iconexperience commented 8 years ago

Fixing the zooming issue would actually be quite easy, we only have to multiply with the pixel ratio when setting the shadow offset and shadow blur to the canvas. Like this (in Item.js lines 3953ff):

ctx.shadowBlur = style.getShadowBlur() * (_pixelRatio || 1);
var offset = this.getShadowOffset();
ctx.shadowOffsetX = offset.x * (_pixelRatio || 1);
ctx.shadowOffsetY = offset.y * (_pixelRatio || 1);

The problem is that at the moment we do not have access to _pixelRatiothere, because it is stored in the view, but we only have access to the canvas context at that place.

Also, I have not tested this on an actual HiDPI environment, but I will.

lehni commented 8 years ago

Yes but we need to fix both issues here (they are not the same)...

lehni commented 8 years ago

While working on the new hit-testing features for #536, I got an idea for this simple and elegant solution. It covers both issues described here and works like a charm for me :)