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-8034] Fix ysld color conversion master #1649

Merged
merged 5 commits into from Nov 15, 2017
Merged

[GEOS-8034] Fix ysld color conversion master #1649

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8adae77
[GEOS-8034] Fixed ysld color conversion
hwbllmnn Jul 12, 2017
5d6c19f
Merge branch 'master' of github.com:geotools/geotools into fix-ysld-c…
hwbllmnn Jul 12, 2017
27a7501
Revert "[GEOS-8034] Fixed ysld color conversion"
hwbllmnn Nov 10, 2017
ab77e3e
fixed ysdl encoder to allow for Color objects
hwbllmnn Nov 10, 2017
98515c1
incorporate requested changes
hwbllmnn Nov 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion modules/extension/ysld/src/main/java/org/geotools/ysld/parse/Util.java
View file
Open in desktop
Expand Up @@ -218,7 +218,13 @@ public static Expression color(Object value, Factory factory) {
if (value != null)
value = value.toString();

return color != null ? factory.filter.literal(color) : expression((String) value, factory);
String hex = null;
if (color != null) {
hex = String.format("#%02x%02x%02x", color.getRed(), color.getGreen(), color.getBlue());
hex = hex.toUpperCase();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay what is this logic trying to do? Is there a bug report? There is a bunch of color converters that cover these cases, would be good to avoid duplication.

Okay updated the description with details from #1648


return color != null ? factory.filter.literal(hex) : expression((String) value, factory);
}

static Color parseColorAsHex(Matcher m) {
Expand Down
14 changes: 6 additions & 8 deletions modules/extension/ysld/src/test/java/org/geotools/ysld/parse/UtilTest.java
View file
Open in desktop
Expand Up @@ -19,8 +19,6 @@

import static org.junit.Assert.assertEquals;

import java.awt.Color;

import org.junit.Test;
import org.opengis.filter.expression.Literal;

Expand All @@ -29,12 +27,12 @@ public class UtilTest {
public void testColor() {
Factory factory = new Factory();

assertEquals(new Color(0xFF0000), ((Literal) Util.color("FF0000", factory)).getValue());
assertEquals(new Color(0x00FF00), ((Literal) Util.color("00ff00", factory)).getValue());
assertEquals(new Color(0x0000FF), ((Literal) Util.color("#0000FF", factory)).getValue());
assertEquals(new Color(0xFFFF00), ((Literal) Util.color("0xFFFF00", factory)).getValue());
assertEquals(new Color(0x00FFFF),
assertEquals("#FF0000", ((Literal) Util.color("FF0000", factory)).getValue());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to make sure that litearls with color objects are still supported, please do not remove the above tests, but you can add to them :)

assertEquals("#00FF00", ((Literal) Util.color("00ff00", factory)).getValue());
assertEquals("#0000FF", ((Literal) Util.color("#0000FF", factory)).getValue());
assertEquals("#FFFF00", ((Literal) Util.color("0xFFFF00", factory)).getValue());
assertEquals("#00FFFF",
((Literal) Util.color("rgb(0, 255, 255)", factory)).getValue());
assertEquals(new Color(0xFF00FF), ((Literal) Util.color("f0f", factory)).getValue());
assertEquals("#FF00FF", ((Literal) Util.color("f0f", factory)).getValue());
}
}
29 changes: 12 additions & 17 deletions modules/extension/ysld/src/test/java/org/geotools/ysld/parse/YsldParseTest.java
View file
Open in desktop
Expand Up @@ -35,7 +35,6 @@
import org.geotools.styling.TextSymbolizer;
import org.geotools.styling.TextSymbolizer2;
import org.geotools.styling.UomOgcMapping;
import org.geotools.ysld.TestUtils;
import org.geotools.ysld.Ysld;
import org.geotools.ysld.YsldTests;
import org.hamcrest.BaseMatcher;
Expand Down Expand Up @@ -67,13 +66,10 @@
import java.io.InputStream;
import java.net.URI;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import javax.measure.unit.SI;

import static org.easymock.EasyMock.expect;
import static org.easymock.classextension.EasyMock.createMock;
import static org.easymock.classextension.EasyMock.replay;
Expand All @@ -87,7 +83,6 @@
import static org.geotools.ysld.TestUtils.lexEqualTo;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.closeTo;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.describedAs;
import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -1189,13 +1184,13 @@ public void testColourMap() throws Exception {
// need to use the geotools.styling interface as it provides the accessors for the entries.
ColorMap map = (ColorMap) symb.getColorMap();

Color colour1 = (Color) map.getColorMapEntry(0).getColor().evaluate(null);
Color colour2 = (Color) map.getColorMapEntry(1).getColor().evaluate(null);
Color colour3 = (Color) map.getColorMapEntry(2).getColor().evaluate(null);
String colour1 = (String) map.getColorMapEntry(0).getColor().evaluate(null);
String colour2 = (String) map.getColorMapEntry(1).getColor().evaluate(null);
String colour3 = (String) map.getColorMapEntry(2).getColor().evaluate(null);

assertThat(colour1, is(Color.RED));
assertThat(colour2, is(Color.GREEN));
assertThat(colour3, is(Color.BLUE));
assertThat(colour1, is("#FF0000"));
assertThat(colour2, is("#00FF00"));
assertThat(colour3, is("#0000FF"));
}

@Test
Expand Down Expand Up @@ -1240,13 +1235,13 @@ public void testColourMapEmpty() throws Exception {
ColorMap map = (ColorMap) symb.getColorMap();

System.out.println(map.getColorMapEntry(0).getColor().evaluate(null));
Color colour1 = (Color) map.getColorMapEntry(0).getColor().evaluate(null);
Color colour2 = (Color) map.getColorMapEntry(1).getColor().evaluate(null);
Color colour3 = (Color) map.getColorMapEntry(2).getColor().evaluate(null);
String colour1 = (String) map.getColorMapEntry(0).getColor().evaluate(null);
String colour2 = (String) map.getColorMapEntry(1).getColor().evaluate(null);
String colour3 = (String) map.getColorMapEntry(2).getColor().evaluate(null);

assertThat(colour1, is(Color.RED));
assertThat(colour2, is(Color.GREEN));
assertThat(colour3, is(Color.BLUE));
assertThat(colour1, is("#FF0000"));
assertThat(colour2, is("#00FF00"));
assertThat(colour3, is("#0000FF"));
}

@SuppressWarnings("unchecked")
Expand Down