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.

Sign up for GitHub

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

Jump to bottom

[GEOS-11324] MapML vector style classes #362

Closed
wants to merge 6 commits into from
Closed

[GEOS-11324] MapML vector style classes #362

wants to merge 6 commits into from

Conversation

turingtestfail
Copy link

Checklist

For core and extension modules:

  • New unit tests have been added covering the changes.
  • Documentation has been updated (if change is visible to end users).
  • The REST API docs have been updated (when changing configuration objects or the REST controllers).
  • There is an issue in the GeoServer Jira (except for changes that do not affect administrators or end users in any way).
  • Commit message(s) must be in the form [GEOS-XYZWV] Title of the Jira ticket.
  • Bug fixes and small new features are presented as a single commit.
  • Each commit has a single objective (if there are multiple commits, each has a separate JIRA ticket describing its goal).

@turingtestfail turingtestfail force-pushed the GEOS-11324-MapML-Vector-Style-Classes branch 7 times, most recently from d8f691c to 8f69db5 Compare March 10, 2024 23:16
@aaime
Copy link
Member

aaime commented Mar 13, 2024

@prushforth this PR attempts to translate SLDs into CSS and apply it to the geometries, however, for what I've tried, we have little to play with. Basic line styles are working, the color of point symbolizes goes through, but for example the fill of polygon does not seem to be applied, and using fill or background-color does not seem to make a difference.

To test, set a layer in "features" mode, without tiling.

Do you have a list of CSS properties the client can work with?

@prushforth
Copy link
Collaborator

Do you have a list of CSS properties the client can work with?

Not really - we are copying classes from the map-feature onto the svg <g> and child <path> that renders it, and including styles in the preceding sibling element as actual <style> elements. The new viewer will work better, I hope. I will test it later today (I'm at work right now and unable to build geoserver or do much useful work beyond email). It's going to be a case of updating the viewer to work with the new GeoServer capabilities in all likelihood, as we haven't had a ready source of vector data before this work.

I will submit a PR to GeoServer main today that I hope to have finished by Friday, but which contains the 0.13.x version of the viewer.

@prushforth
Copy link
Collaborator

Currently, the PR is adding class values to the root geometry type element:

image

If you put the class on the <map-feature> instead, it will apply the styles you're sending:

image

@prushforth
Copy link
Collaborator

I apologize that our documentation of this feature is not up to date - that's my fault. It's also an area that we haven't had much call to work on, but having a ready source of styled vector content will definitely help motivate us and will shape the viewer's future capabilities.

@turingtestfail turingtestfail force-pushed the GEOS-11324-MapML-Vector-Style-Classes branch from 4473fa4 to 81f1e0f Compare March 14, 2024 14:45
@turingtestfail
Copy link
Author

I apologize that our documentation of this feature is not up to date - that's my fault. It's also an area that we haven't had much call to work on, but having a ready source of styled vector content will definitely help motivate us and will shape the viewer's future capabilities.

No worries at all. It was straightforward to move the style class identifier up to the feature. The latest push is ready for your testing.

@turingtestfail
Copy link
Author

I apologize that our documentation of this feature is not up to date - that's my fault. It's also an area that we haven't had much call to work on, but having a ready source of styled vector content will definitely help motivate us and will shape the viewer's future capabilities.

No worries at all. It was straightforward to move the style class identifier up to the feature. The latest push is ready for your testing.

Also, please note that there was a naming collision in Feature - the featuretype was being written to a "clazz" attribute that was aliased to "class". I changed that alias to "featuretype-class".

@prushforth
Copy link
Collaborator

Looking pretty good now - note that the viewer has hard-coded the svg stroke- attributes and this is causing some competition with the <LineSymbolizer> rules

image

I will see if I can fix that asap.

@prushforth
Copy link
Collaborator

I believe the features are being duplicated - once for each symbolizer perhaps?:

image

@aaime
Copy link
Member

aaime commented Mar 14, 2024

That's by design, in SLD each symbolizer paints on top of the previous one, to generate visual effects like cased roads, complex symbols and the like. Can this be achieved without duplicating features?

@prushforth
Copy link
Collaborator

I think so, just put the symbolizers in the class attribute in order of rendering?

@turingtestfail
Copy link
Author

turingtestfail commented Mar 14, 2024 via email

@aaime
Copy link
Member

aaime commented Mar 14, 2024

I think so, just put the symbolizers in the class attribute in order of rendering?

That's surprising... so I can put a stroke twice, with two different colors and two different sizes, to get an effect like this one:

image

In CSS that's not possible, the second stroke definition would overwrite the first. Is SVG working differently?

@prushforth
Copy link
Collaborator

Can this be achieved without duplicating features?

Is SVG working differently?

Our renderer is generating SVG so its the renderer that is doing the duplication. Part of the goal of this project is to find the limits of what is possible on the Web, especially in CSS wrt maps and push that as a change request.

@turingtestfail
Copy link
Author

turingtestfail commented Mar 14, 2024 via email

@prushforth
Copy link
Collaborator

Should I delimit multiple classes with spaces, similar to CSS?

Yes, please

@turingtestfail
Copy link
Author

turingtestfail commented Mar 14, 2024 via email

@turingtestfail
Copy link
Author

turingtestfail commented Mar 14, 2024 via email

@prushforth
Copy link
Collaborator

For the feature representation, should there
be a way to pass scale denominators/zoom layering information to the client
from the SLD Rules? If so where would you want to put that information in
the XML?

It's a good question, and I'm not sure I fully understand it. We think CSS will have a major role in deciding (at least, applying specified rules for) the scale / zoom ranges within which a feature should be rendered and with what properties. However that is getting out beyond the forseeable horizon.

In the meantime, we spent A LOT of time working on the <map-feature zoom="..." min="..." max="..."> attributes.

The zoom attribute says that what the "native" zoom/scale of the feature is. The min and max attributes dictate for now what the display scale range of a feature is, at the per-feature level. When you have a <map-link rel="features"> it pulls in features at each map movement, and the zoom property of the feature, if not actually specified is set from the zoom of the map at the moment the feature joins the fun. The min and max values (again, which specify only the visible zoom range), if not specified on the feature as an attribute, are dictated by the <map-input type=zoom min="..." max="..."> values, or failing that by a fallback search for relevant values up to and including the applicable values from the projection.

So, long story short, we might eventually want to create a <map-link media="..."> attribute that contains a CSS media query that pertains to the map zoom to allow the viewer to automatically select which <map-link> to load based on the viewer zoom situation. In other words, no action is required at this time! I think!

@prushforth
Copy link
Collaborator

prushforth commented Mar 14, 2024 •
edited
Loading

Looks pretty nice - less competition with the stroke-* attributes for sure:

image

I don't think we need that featuretype-class="whatever" attribute actually. Thanks!

Also, no map-feature duplicates, looks good!

@aaime
Copy link
Member

aaime commented Mar 15, 2024

It seems this one is ready to become a PR for the official repository. Can you confirm?

@prushforth
Copy link
Collaborator

Good to go!

@turingtestfail turingtestfail force-pushed the GEOS-11324-MapML-Vector-Style-Classes branch from cde6456 to a1af45d Compare March 15, 2024 12:35
@turingtestfail turingtestfail force-pushed the GEOS-11324-MapML-Vector-Style-Classes branch from bb08ea6 to 8f13959 Compare March 15, 2024 12:45
@turingtestfail
[GEOS-11324] MapML WMS Vector Representation Style Classes
6d5e056 
started on point sld conversion

started on point radius size

First pass at line styles

added fill opacity

added logging

added tests and fixed issues for style visitor

cleanup

added cql-filter to sld filter test

cleanup

class text

assigned class to features and added geom conversion

added some comments and logging

added tests and point to polygon

cleanup

fixed issue with null styles

switched to midpoint

moved style class to geometry and cleaned up style text spacing

fixed test with new style spacing and leading dot

moved class identifier to feature

switched to one feature per symbolizer

removed featuretype-class population

misc cleanup

more cleanup
@turingtestfail turingtestfail force-pushed the GEOS-11324-MapML-Vector-Style-Classes branch from 8f13959 to 6d5e056 Compare March 15, 2024 19:03
@prushforth
Copy link
Collaborator

Joe - thanks for working on the scale-dependent symbolizers. I have tried it locally (rebased on main, actually).

The following modified style rules for the states shapefile's population.sld (CBMTILE projection):

 <Rule>
          <Title>&gt; 4M</Title>
          <!-- like a linesymbolizer but with a fill too -->
          <ogc:Filter>
            <ogc:PropertyIsGreaterThan>
             <ogc:PropertyName>PERSONS</ogc:PropertyName>
             <ogc:Literal>4000000</ogc:Literal>
            </ogc:PropertyIsGreaterThan>
          </ogc:Filter>
          <MinScaleDenominator>2000</MinScaleDenominator>
          <MaxScaleDenominator>5000000</MaxScaleDenominator>          <PolygonSymbolizer>
             <Fill>
                <!-- CssParameters allowed are fill (the color) and fill-opacity -->
                <CssParameter name="fill">#4df7ff</CssParameter>
                <CssParameter name="fill-opacity">0.7</CssParameter>
             </Fill>     
          </PolygonSymbolizer>
        </Rule>
        <Rule>
          <Title>&gt; 4M</Title>
          <!-- like a linesymbolizer but with a fill too -->
          <ogc:Filter>
            <ogc:PropertyIsGreaterThan>
             <ogc:PropertyName>PERSONS</ogc:PropertyName>
             <ogc:Literal>4000000</ogc:Literal>
            </ogc:PropertyIsGreaterThan>
          </ogc:Filter>
          <MinScaleDenominator>5000000</MinScaleDenominator>
                    <PolygonSymbolizer>
             <Fill>
                <!-- CssParameters allowed are fill (the color) and fill-opacity -->
                <CssParameter name="fill">#4D4DFF</CssParameter>
                <CssParameter name="fill-opacity">0.7</CssParameter>
             </Fill>     
          </PolygonSymbolizer>
</Rule>

works like this in the style previewer:

Recording.2024-03-17.100519.mp4

but the max scale denominator does not work quite as it does in the openlayers style preview:

Recording.2024-03-17.101108.mp4

@prushforth
Copy link
Collaborator

If I reverse the order of the rules it seems that it only picks up and applies the first rule.

@turingtestfail
Copy link
Author

turingtestfail commented Mar 17, 2024 via email

@prushforth
Copy link
Collaborator

It works much better now! Thanks for your quick action. Probably won't get another look at it until tonight/tomorrow.
Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@turingtestfail @aaime @prushforth