-
Notifications
You must be signed in to change notification settings - Fork 120
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
Added raster-
(hue-rotate
/contrast
/saturation
/opacity
/brightness-*
/) as operations
#1055
base: main
Are you sure you want to change the base?
Conversation
if (!id || id != glSourceId) { | ||
if ( | ||
// This line is because rasters set properties on their source | ||
glLayer.type === 'raster' || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the line I'm most unsure about. We need this because is we have two layers with the same source they need to apply different operations to their sources.
raster-
(hue-rotate
/saturation
/opacity
) as operationsraster-
(hue-rotate
/opacity
) as operations
Tests are failing due to the addition of |
I'm sorry, but I won't be able to review this for a while. I'm terribly busy at the moment. |
No worries @ahocevar |
raster-
(hue-rotate
/opacity
) as operationsraster-
(hue-rotate
/hue-contrast
/hue-saturation
/opacity
/brightness-*
/) as operations
raster-
(hue-rotate
/hue-contrast
/hue-saturation
/opacity
/brightness-*
/) as operationsraster-
(hue-rotate
/contrast
/saturation
/opacity
/brightness-*
/) as operations
Updated this to support more of the spec, if you are happy I'll fix up the tests. |
I'm failing to understand why existing tests are failing - RasterSource was already there before. At the moment I don't have time to give it a closer look, but if you are able to add a commit that helps understand what you think is wrong with the tests, it would help. |
@ahocevar I'm still doing some hunting on this one, hence still in draft. Essentially previously a raster was a
Now it's a
So essentially there is this new wrapper, where const layer = new ImageLayer({
source: new Raster({
operationType: 'image',
operation: rasterShader,
sources: [tileLayer],
}),
}); So the source in the following line of the tests now refers to the it('creates the correct tile grid for raster sources', function (done) {
apply(target, context)
.then(function (map) {
const statesSource = map.getLayers().item(0).getSource(); // <======== HERE
const statesTileGrid = statesSource.getTileGrid();
should(statesTileGrid.getTileSize()).eql(256);
should(statesTileGrid.getExtent()).eql([
-20037508.342789244, -20037508.342789244, 20037508.342789244,
20037508.342789244,
]); Seems like an easy fix right? But I can't appear to find the openlayers API to get to the const layer = new ImageLayer({
source: new Raster({
operationType: 'image',
operation: rasterShader,
sources: [tileLayer], // <=== Not sure how to get at this from the test suite.
}),
}); I'm happy doing some more hunting, but it'll probably be a couple of days, as I'm a little busy right now. |
If I understand you correctly, a |
Yeah that makes sense, I'll add a branch for raster without operations. I'll try and add some new tests for the raster operations as well. |
This PR adds the following styling rules
raster-hue-rotate
raster-contrast
raster-saturation
raster-brightness-min
raster-brightness-max
Missing: Tests for new
raster-*
operations.