Remove 0.75x downscaling of SVG icons #7118

dvdmrtnz · 2024-01-08T17:46:04Z

dvdmrtnz commented

2024-01-08 17:46:04 +00:00

This PR removes the 0.75x downscaling of the SVG icons of POI in the current SVG→PNG workflow. This will simplify work for icon designers as they will be able to use a 1px grid to design the icons that will match the final pixel grid of the icons in the app.

I have modified all the icons to take into account the removal of this resize, so they look the same size in the app. However, current icons will have to be tweaked to take advantage of this change and make them match the grid. That is, unless they were designed already with the previous 1pt=0.75px conversion in mind. In that case they will match the new grid just fine.

Fixes section 3 of organicmaps/organicmaps#4017

This PR removes the 0.75x downscaling of the SVG icons of POI in the current SVG→PNG workflow. This will simplify work for icon designers as they will be able to use a 1px grid to design the icons that will match the final pixel grid of the icons in the app. I have modified all the icons to take into account the removal of this resize, so they look the same size in the app. However, current icons will have to be tweaked to take advantage of this change and make them match the grid. That is, unless they were designed already with the previous 1pt=0.75px conversion in mind. In that case they will match the new grid just fine. * Fixes section 3 of https://git.omaps.dev/organicmaps/organicmaps/issues/4017

euf (Migrated from github.com) approved these changes 2024-01-09 16:21:30 +00:00

vng (Migrated from github.com) approved these changes 2024-01-14 16:32:08 +00:00

skin_generator/generator.cpp

					
				@ -88,10 +93,10 @@ void SkinGenerator::ProcessSymbols(std::string const & svgDataDir,

				        QString fullFileName = QString(dir.absolutePath()) + "/" + fileName;

				        if (m_svgRenderer.load(fullFileName))

				        {

vng (Migrated from github.com) commented

2024-01-14 16:32:04 +00:00

Can we clarify here where the 18 constant comes from?
Is it something connected with m_svgRenderer.defaultSize (a factor of 18)?
Should we still keep this transformation when we adjusted all the svg dimensions (in later commits)?

Can we clarify here where the 18 constant comes from? Is it something connected with m_svgRenderer.defaultSize (a factor of 18)? Should we still keep this transformation when we adjusted all the svg dimensions (in later commits)?

pastk approved these changes 2024-01-14 16:37:03 +00:00

pastk left a comment

Did anyone test the last version in an app build?

skin_generator/generator.cpp

					
				@ -88,10 +93,10 @@ void SkinGenerator::ProcessSymbols(std::string const & svgDataDir,

				        QString fullFileName = QString(dir.absolutePath()) + "/" + fileName;

				        if (m_svgRenderer.load(fullFileName))

				        {

pastk commented

2024-01-14 16:34:29 +00:00

I don't understand it too and it'll be great to clarify this logic.

Not necessary in this PR.

(I wanted to go through the code and learn it myself, but don't have time now)

I don't understand it too and it'll be great to clarify this logic. Not necessary in this PR. (I wanted to go through the code and learn it myself, but don't have time now)

dvdmrtnz reviewed 2024-01-14 17:53:14 +00:00

skin_generator/generator.cpp

					
				@ -88,10 +93,10 @@ void SkinGenerator::ProcessSymbols(std::string const & svgDataDir,

				        QString fullFileName = QString(dir.absolutePath()) + "/" + fileName;

				        if (m_svgRenderer.load(fullFileName))

				        {

dvdmrtnz commented

2024-01-14 17:53:14 +00:00

As far as I know this is how this code works:

m_svgRenderer.defaultSize() is the size of the SVG file (After the resize in this PR generally the -m files are 18x18 and the -s files are 12x12 ─ previously -m files were 24x24 and -s files were 16x16)
symbolSize is the size of the icons in the PNG skins that we want to generate and they are defined in the script generate_symbols.sh: 18 (mdpi), 27 (hdpi), 36 (xhdpi)...
The 18.0 comes from the fact that the icons that should be represented as 18x18 in mdpi are also saved as 18x18 SVG files. Previously the constant 24.0 came from the fact that the icons that should be represented as 18x18 in mdpi were saved as 24x24 SVG files.

As far as I know this is how this code works: * `m_svgRenderer.defaultSize()` is the size of the SVG file (After the resize in this PR generally the -m files are 18x18 and the -s files are 12x12 ─ previously -m files were 24x24 and -s files were 16x16) * `symbolSize` is the size of the icons in the PNG skins that we want to generate and they are defined in the script [generate_symbols.sh](https://github.com/organicmaps/organicmaps/blob/master/tools/unix/generate_symbols.sh#L82-L94): 18 (mdpi), 27 (hdpi), 36 (xhdpi)... * The `18.0` comes from the fact that the icons that should be represented as 18x18 in mdpi are also saved as 18x18 SVG files. Previously the constant `24.0` came from the fact that the icons that should be represented as 18x18 in mdpi were saved as 24x24 SVG files.

pastk reviewed 2024-01-14 18:01:04 +00:00

skin_generator/generator.cpp

					
				@ -88,10 +93,10 @@ void SkinGenerator::ProcessSymbols(std::string const & svgDataDir,

				        QString fullFileName = QString(dir.absolutePath()) + "/" + fileName;

				        if (m_svgRenderer.load(fullFileName))

				        {

pastk commented

2024-01-14 18:01:04 +00:00

Many thanks! Could you please convert this knowledge into the code comments?

dvdmrtnz reviewed 2024-01-14 18:31:58 +00:00

skin_generator/generator.cpp

					
				@ -88,10 +93,10 @@ void SkinGenerator::ProcessSymbols(std::string const & svgDataDir,

				        QString fullFileName = QString(dir.absolutePath()) + "/" + fileName;

				        if (m_svgRenderer.load(fullFileName))

				        {