[drape] Draw embankments. #2738

pastk · 2022-06-14T14:24:03Z

vng commented

2022-06-14 14:24:03 +00:00

(Migrated from github.com)

pastk reviewed 2022-06-14 16:58:23 +00:00

data/styles/clear/include/Basemap.mapcss

					
				@ -701,23 +701,25 @@ line|z18-[barrier=hedge],

				line|z18-[barrier=retaining_wall],

				line|z18-[barrier=wall],

				line|z18-[historic=citywalls],

pastk commented

2022-06-14 16:58:23 +00:00

please add a comment explaining meaning of dashes: 6,4,4,2;
so that others can use the feature for other objects later

please add a comment explaining meaning of `dashes: 6,4,4,2;` so that others can use the feature for other objects later

pastk reviewed 2022-06-14 16:58:42 +00:00

data/styles/clear/include/Basemap.mapcss

					
				@ -701,23 +701,25 @@ line|z18-[barrier=hedge],

				line|z18-[barrier=retaining_wall],

				line|z18-[barrier=wall],

				line|z18-[historic=citywalls],

pastk commented

2022-06-14 16:58:42 +00:00

also would be great to make it dark-brown :)

biodranik (Migrated from github.com) approved these changes 2022-06-15 06:13:58 +00:00

data/styles/clear/include/Basemap.mapcss

					
				@ -701,23 +701,25 @@ line|z18-[barrier=hedge],

				line|z18-[barrier=retaining_wall],

				line|z18-[barrier=wall],

				line|z18-[historic=citywalls],

biodranik (Migrated from github.com) commented

2022-06-14 20:49:26 +00:00

Как забор красить точно не нужно.

drape/stipple_pen_resource.cpp

					
				@ -15,3 +14,3 @@

				uint32_t const kStippleHeight = 1;

				uint32_t const kMaxStipplePenLength = 512;  /// @todo Should be equal with kStippleTextureWidth?

				StipplePenPacker::StipplePenPacker(m2::PointU const & canvasSize)

biodranik (Migrated from github.com) commented

2022-06-14 16:00:44 +00:00

Так сразу пропиши equal.

drape/texture_manager.cpp

					
				@ -39,2 +41,4 @@

				// Reserved for elements like RuleDrawer or other LineShapes.

				uint32_t const kReservedPatterns = 0;

				size_t const kReservedColors = 20;

biodranik (Migrated from github.com) commented

2022-06-14 20:54:19 +00:00

Если сомневаешься, сделай сразу равной kMaxStipplePenLength, и оставь туду.

drape/texture_manager.cpp

biodranik (Migrated from github.com) commented

2022-06-14 21:03:37 +00:00

Тут на && или ниже на empace бы не ускорили чуток инициализацию?

drape_frontend/line_shape.cpp

biodranik (Migrated from github.com) commented

2022-06-15 06:13:10 +00:00

Нехорошо, может метод неконстантным сделать? А то неочевидно что у конст объекта что-то меняется.

vng (Migrated from github.com) reviewed 2022-06-15 06:39:03 +00:00

drape/stipple_pen_resource.cpp

					
				@ -15,3 +14,3 @@

				uint32_t const kStippleHeight = 1;

				uint32_t const kMaxStipplePenLength = 512;  /// @todo Should be equal with kStippleTextureWidth?

				StipplePenPacker::StipplePenPacker(m2::PointU const & canvasSize)

vng (Migrated from github.com) commented

2022-06-15 06:39:03 +00:00

Они сейчас equal, я оставляю вопрос, т.к. кажется что константы одной природы, но не уверен.

vng (Migrated from github.com) reviewed 2022-06-15 06:39:25 +00:00

drape/texture_manager.cpp

					
				@ -39,2 +41,4 @@

				// Reserved for elements like RuleDrawer or other LineShapes.

				uint32_t const kReservedPatterns = 0;

				size_t const kReservedColors = 20;

vng (Migrated from github.com) commented

2022-06-15 06:39:25 +00:00

Они равны, тут вопрос

vng (Migrated from github.com) reviewed 2022-06-15 06:39:48 +00:00

drape/texture_manager.cpp

vng (Migrated from github.com) commented

2022-06-15 06:39:48 +00:00

нет

vng (Migrated from github.com) reviewed 2022-06-15 06:40:02 +00:00

drape_frontend/line_shape.cpp

vng (Migrated from github.com) commented

2022-06-15 06:40:02 +00:00

я наверное уберу этот хак

vng commented

2022-06-15 06:41:14 +00:00

(Migrated from github.com)

Как красить embankment? Вот я смотрел в вики ОСМ, везде серый цвет.
https://wiki.openstreetmap.org/wiki/File:Natural_cliff-125.png
https://wiki.openstreetmap.org/wiki/File:Man_made_embankment.png

Как красить embankment? Вот я смотрел в вики ОСМ, везде серый цвет. https://wiki.openstreetmap.org/wiki/File:Natural_cliff-125.png https://wiki.openstreetmap.org/wiki/File:Man_made_embankment.png

vng commented

2022-06-15 08:20:24 +00:00

(Migrated from github.com)

Second screen with embankment like path color:
Screenshot 2022-06-15 at 11 19 37

Second screen with embankment like path color: <img width="1416" alt="Screenshot 2022-06-15 at 11 19 37" src="https://user-images.githubusercontent.com/175612/173779008-2068733e-d874-4e99-87b5-52362aa83bc4.png">

vng commented

2022-06-15 14:00:57 +00:00

(Migrated from github.com)

Latest version:
Screenshot 2022-06-15 at 17 00 06

Latest version: <img width="1152" alt="Screenshot 2022-06-15 at 17 00 06" src="https://user-images.githubusercontent.com/175612/173846206-6b936f1d-b16e-4d0f-aa42-9411eb65f3c0.png">

pastk reviewed 2022-06-15 14:18:53 +00:00

data/styles/clear/include/Basemap.mapcss

pastk commented

2022-06-15 14:18:53 +00:00

Its not clear how "base height" is different from the thickness of the line (dash)...

Does the renderer really make breaks between the "dashes" to fit triangles into instead of just overlaying them?

I imagined it looks more like

================= - a line
    \/     \/     - triangles

Its not clear how "base height" is different from the thickness of the line (dash)... Does the renderer really make breaks between the "dashes" to fit triangles into instead of just overlaying them? I imagined it looks more like ``` ================= - a line \/ \/ - triangles ```

vng (Migrated from github.com) reviewed 2022-06-15 14:22:12 +00:00

data/styles/clear/include/Basemap.mapcss

vng (Migrated from github.com) commented

2022-06-15 14:22:11 +00:00

base height is the thickness of the dash line

pastk reviewed 2022-06-15 14:56:23 +00:00

data/styles/clear/include/Basemap.mapcss

pastk commented

2022-06-15 14:56:23 +00:00

whys is it different from width: 7.0; then? (base height is just 2)

whys is it different from `width: 7.0;` then? (base height is just 2)

vng (Migrated from github.com) reviewed 2022-06-15 15:14:21 +00:00

data/styles/clear/include/Basemap.mapcss

vng (Migrated from github.com) commented

2022-06-15 15:14:20 +00:00

"width" is a total pattern height = 5 + 2

pastk reviewed 2022-06-15 16:24:56 +00:00

data/styles/clear/include/Basemap.mapcss

pastk commented

2022-06-15 16:24:56 +00:00

Ok, thanks, I understand now - could you please add it to the comment?

Also what (bad) will happen if width and base height + triangle height are inconsistent? Maybe its possible not to have an extra parameter if it could be derived from other params?

Is it technically hard to make it (imho) more logical like
width - line width w/o triangles
dashes: dash-length, pattern-base-width, pattern-height
?

(also I don't really like overloading the dashes: with something that doesn't look like a dashed line, but maybe its too much to ask)

Ok, thanks, I understand now - could you please add it to the comment? Also what (bad) will happen if `width` and `base height + triangle height` are inconsistent? Maybe its possible not to have an extra parameter if it could be derived from other params? Is it technically hard to make it (imho) more logical like `width` - line width w/o triangles `dashes: dash-length, pattern-base-width, pattern-height` ? (also I don't really like overloading the `dashes:` with something that doesn't look like a dashed line, but maybe its too much to ask)

vng (Migrated from github.com) reviewed 2022-06-15 16:37:36 +00:00

data/styles/clear/include/Basemap.mapcss

vng (Migrated from github.com) commented

2022-06-15 16:37:36 +00:00

Yes, this is not about "dashes" pattern, but it was the optimal way to implement at this moment.
Will reconsider style definitions, when "pattern" line drawing will be more varied and complicated.

Yes, this is not about "dashes" pattern, but it was the optimal way to implement at this moment. Will reconsider style definitions, when "pattern" line drawing will be more varied and complicated.

This repo is archived. You cannot comment on pull requests.

No reviewers

biodranik