Move gradient and pattern fills with the view #5950
Conversation
|
I love the effect! And the name makes sense as does the place where you suggest to set it. I like it! |
|
And can we somehow get the same with the gradient or am I missing sth.? |
That would require more changes to the example (different start and end coordinates for the gradient), but should also work with the new option. I did not apply it to the gradient because I don't see much use in filling polygons with a gradient. |
|
I wonder if the current behaviour is needed at all, or if we should just make this the default? |
|
Generally I agree, Bart. Why not add the option and default it to true? That would be a behavioural change but easily revertable by our users. Though I doubt many actually use this feature. |
|
I'd be tempted to call it a bug fix (instead of exposing an option). |
ahocevar
force-pushed
the
fill-movewithview
branch
2 times, most recently
from
1492eec
to
c29dee5
Compare
|
I updated the pull request according to the suggestions and made the new behavior the default, without a new option to change to the previous behavior. I also changed the example to use a per-feature gradient instead of a global one, which probably makes more sense:
|
|
Very nice, especially the pixelRatio usage in the example. The explanation there could probably have one sentence about that, but I'll leave it up to you. |
ahocevar
changed the title
| canvas.height = 11 * pixelRatio; | ||
| // white background | ||
| context.fillStyle = 'white'; | ||
| context.fillRect(0, 0, 11 * pixelRatio, 11 * pixelRatio); |
There was a problem hiding this comment.
minor but could use canvas.width and canvas.height here
| * panning or rotating. Default is `false`. | ||
| * @type {boolean} | ||
| */ | ||
| olx.style.FillOptions.prototype.moveWithView; |
| @@ -208,15 +208,18 @@ ol.renderer.canvas.TileLayer.prototype.forEachLayerAtPixel = function( | |||
| }; | |||
|
|
|||
|
|
|||
| ol.renderer.canvas.TileLayer.prototype.renderTileImage = function(replayContext, frameState, tile, tileGrid, tilePixelRatio, skippedFeatureUids) { | |||
|
|
|||
| }; | |||
There was a problem hiding this comment.
why is this empty function needed?
There was a problem hiding this comment.
Accidently committed, part of my next pull request 😄 - removed.
ahocevar
force-pushed
the
fill-movewithview
branch
2 times, most recently
from
dcfdf41
to
31532ba
Compare
ahocevar
force-pushed
the
fill-movewithview
branch
from
31532ba
to
f655f67
Compare
|
Thanks for the comments and reviews! |
Opened for discussion.
Currently, when using a gradient or pattern to fill polygons, gradient and pattern won't change their position when panning/rotating the map. See https://openlayers.org/en/master/examples/canvas-gradient-pattern.html.
To change this, I have added a
moveWithViewoption tool.style.Fill. When set to true, fills will move and rotate with the view. This is especially useful for pattern fills, as can be seen here:moveWithView: false:moveWithView: true:Any opinions on whether this is the way to go, or if we should solve this in a different way?