Render raceways highways
ClosedPublic

Authored by xdizzaster on Dec 2 2016, 5:24 PM.

Details

Reviewers
nienhueser
rahn
Group Reviewers
Marble

Diff Detail

Repository
R34 Marble
Lint
Lint Skipped
Unit
Unit Tests Skipped
xdizzaster updated this revision to Diff 8701.Dec 2 2016, 5:24 PM
xdizzaster retitled this revision from to Render raceways highways.
xdizzaster updated this object.
xdizzaster edited the test plan for this revision. (Show Details)
xdizzaster set the repository for this revision to R34 Marble.
rahn added a subscriber: rahn.
rahn edited edge metadata.Dec 2 2016, 7:32 PM

Nice, still some minor nitpicks.

src/lib/marble/StyleBuilder.cpp
154

This value marks the minimum zoom level where the street appears. Zoomlevels are exactly organized like in OpenStreetMap: level 0 is the lowest one which contains the whole earth in a single picture. Level 19 the highest one where you can see all details of a small street.
As you can see the biggest streets only appear at level 7. Raceways are less prominent in general so they should probably rather appear around level 11 or 13.

732

Should we really not display any outline?

xdizzaster marked 2 inline comments as done.Dec 2 2016, 7:58 PM

You are probably right.

src/lib/marble/StyleBuilder.cpp
154

I had trouble with displaying it on the higher zoom levels, but I suspect it was because of the way my example (https://www.openstreetmap.org/way/27852990) was rendered in particular. i have checked on OSM and the lowest zoom level I can see the raceway is 12.

732

We probably should, my bad.

xdizzaster updated this revision to Diff 8705.Dec 2 2016, 8:00 PM
xdizzaster edited edge metadata.
xdizzaster marked 2 inline comments as done.

Changed the zoom level to a more reasonable minimum and added an outline to the line

xdizzaster marked 2 inline comments as done.Dec 2 2016, 8:01 PM
rahn added a comment.Dec 2 2016, 8:48 PM

Sorry, still a nitpick: during compilation I get this:

/home/tackat/marble/sources/src/lib/marble/geodata/data/GeoDataPlacemark.cpp: In member function ‘QString Marble::GeoDataPlacemark::categoryName() const’:
/home/tackat/marble/sources/src/lib/marble/geodata/data/GeoDataPlacemark.cpp:362:12: warning: enumeration value ‘HighwayRaceway’ not handled in switch [-Wswitch]

switch (d->m_visualCategory) {
rahn added a comment.Dec 2 2016, 8:53 PM

I've tested you patch and the only thing left to complete this task seems to be the compilation warning above. Apart from that it looks great.

xdizzaster updated this revision to Diff 8708.Dec 2 2016, 9:13 PM

Fixed a warning about an unhandled switch case.

rahn accepted this revision.Dec 2 2016, 9:19 PM
rahn edited edge metadata.

Excellent!

This revision is now accepted and ready to land.Dec 2 2016, 9:19 PM
nienhueser closed this revision.Dec 3 2016, 10:11 PM
nienhueser edited edge metadata.

Submitted