Order landcover by osm_id

[matthijsmelissen]

This resolves #2782

[andrzej-r]

Do you thing it would be possible to sort by feature instead. I understand sorting by osm_id would produce different rendering when someone deletes/recreates an object, which seems like a hack to me.

[matthijsmelissen]

Do you thing it would be possible to sort by feature instead.

Hmm, good point. I've been thinking about it, but I think it would add quite a lot of code for a relatively small gain. So I think I'd prefer to leave it as it.

[sommerluk]

it would add quite a lot of code

Wouldn’t it be enough to leave the inner query as it is currently (without this PR), and add to the outer query ORDER BY feature, osm_id? This should order by feature alphabetically (which is just as arbitrary as osm_id, but independent from deleting/re-creating objects in the OSM database) before ordering by osm_id…

[matthijsmelissen]

My bad, this would work of course. Will fix it.

[sommerluk]

Hm, my bad, should be ORDER BY way_area DESC, feature, osm_id on the outer query (and no order in the inner query), otherwise way_area wouldn’t be the primary order criterium any more.

[pnorman]

I don't see that we need osm_id ordering if we've got feature in the ordering

[sommerluk]

I don't see that we need osm_id ordering if we've got feature in the ordering

Good point.

[ImreSamu]

I don't see that we need osm_id ordering if we've got feature in the ordering

imho - the original varied color problem is solved, but it can be a similar problem in the varied landcover labels. ( in rare cases )

[pnorman]

imho - the original varied color problem is solved, but it can be a similar problem in the varied landcover labels. ( in rare cases )

The landcover layer is used for fills, not labels.

[kocio-pl]

What is current state of this code and what are the plans to do with it?

[pnorman]

It needs changes to stop ordering after feature.

[Tomasz-W]

It resolves #1291 also.

[kocio-pl]

@matthijsmelissen What would you like to do with code? Do you have any plans?

[matthijsmelissen]

Updated.

[kocio-pl]

I'm not sure if you've seen this, but there's 1 conflict left (related to #2874).

[matthijsmelissen]

Superseded by #3441.

The current PR is a PR from a branch on gravitystorm directly, it should of course come from matthijsmelissen:landcover, which is fixed in the new PR.