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.

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

Cluttered ways #1220

Open
vyskocil opened this issue Feb 24, 2022 · 27 comments
Open

Cluttered ways #1220

vyskocil opened this issue Feb 24, 2022 · 27 comments
Labels
feature For issues and pull request that request or implement a new feature renderer For issues in the content of the rendering engine

Comments

@vyskocil
Copy link
Collaborator

Way rendering maybe cluttered, especially the one way as you could see in the attached screenshot the way labels and one way arrows often overlaps. Is there a solution to avoid this issue ?
Maybe the way rendering code should treat the labels and the images in the same process allowing to alternate both along the path ?
Another simpler solution could be to provide the background color of the way to the label renderer, thus it could use it to draw the text with opaque background and overdraw the one way arrows.

Cluttered ways

@vyskocil
Copy link
Collaborator Author

Here is the standard OpenStreetMap rendering of the same area :

Screenshot 2022-02-24 at 11 23 46

@Karry Karry added feature For issues and pull request that request or implement a new feature renderer For issues in the content of the rendering engine labels Mar 28, 2022
@Framstag
Copy link
Owner

Framstag commented Jul 2, 2022

This (coordinated rendering of symbols ad labels) should in principle possible, however would need some changes.

Currently labels are handled by DrawWayContourLabel, with positioning done in the LabelLayouter.RegisterContourLabel method, based on parameter passed in MapPainter::DrawWayContourLabel.

For symbols this is triggered in MapPainter::DrawWayDecoration with the actual positioning in MapPainterXXX.DrawContourLabel()

In both cases, start and offset to the next instance just would to be input from the outside (and calculated for both coordinated), instead being calculated separately.

@Framstag
Copy link
Owner

@vyskocil

The placement for the contour symbols has now been moved (in principle, did not check all backends - only cairo and Qt) from the individual backend code to the MapPainter.
Similar refactoring would have to be done now for the label placement of the way labels.
Finally, this would allow coordinating the placement of labels and contour symbols.

Note, that contour symbols now can be scaled in height to the width of the way.

If some of my refactoring broke the IOSX backend, please contact me. This should be easy to fix, just make sure to evaluate alle the information in SymbolContourData.

@vyskocil
Copy link
Collaborator Author

Hi @Framstag ! Thank you very much taking this issue into account !
I just tried the up to date code just before the scalable contour symbol which is not merged yet, there is indeed a lot less clutter as in my case there is no more way labels at all... ;-) Maybe there are some parameters to tweak in the stylesheet file (I don't use the one provided with the code) ?

Here is a screenshot of the same place :

IMG_EBE6AB3F66D5-1

@vyskocil
Copy link
Collaborator Author

Looking further on the map rendered on iOS I couldn't find any way label now, even for the no one way roads... What should I look for to debug this issue ?

@Framstag
Copy link
Owner

@vyskocil AFAIK I did not do any principle changes to the text handling. I had to change/enhance a few APIs regarding Symbols but that should not have had any influence on the text APIs.

Since I normally had to "poke" the source for IOS without Syntax higlighting, local build or testing, it is possible that I broke something. I suggest you check my change around the LabelLayouter to check, if I did a wrong replacement that compiled but broke something.

Since I do not regularly check to correctness of all the backends, it is possible, though, that it broke before.

Sorry for that possible inconvenience.

Yes, still sorting out build errors, may take some time into the week to get that fixed...time is already running out :-/

@vyskocil
Copy link
Collaborator Author

@Framstag regarding the scalable contour symbol, I'm trying to add the needed code to support this option for iOS/OSX backend but I'm having an issue.
I modified void MapPainterIOS::DrawContourSymbol() to pass the new scaleFactor argument to DrawSymbol :

...
DrawSymbol(projection, parameter, symbol, 0, 0, data.symbolScale);
...

But in DrawSymbol code I only receive 1.0 or 0.0 values for scaleFactor !

I also modified the stylesheet file with :

   WAY.SYMBOL { symbol: oneway_arrow; symbolSpace: 10mm; renderMode: scale; scale: 0.5;}

@Framstag
Copy link
Owner

@vyskocil

The necessary algorithm is implemented in MapPainter::DrawWayDecoration (else branch => more refactoring needed ;-)):

        if (pathSymbolStyle->GetRenderMode()==PathSymbolStyle::RenderMode::scale) {
          symbolScale=data.mainSlotWidth*pathSymbolStyle->GetScale()/symbolBoundingBox.GetHeight();
        }
        else {
          symbolScale=1.0;
        }

The stylesheet entry should result in the if-branch getting entered.

It sets symbolScale and then calls DrawContourSymbol(). DrawContourSymbol itself is driver specific and must handle the scale factor. You can implement SymbolRenderer and delegate rendering to it, but you do not need to (see the cairo version to see, how I integrated the transformation code).

Hope this helps.

@vyskocil
Copy link
Collaborator Author

@Framstag does that mean that I have to wait the other branch is refactored and merged to receive the good symbolScale in the iOS driver DrawContourSymbol method ?

@Framstag
Copy link
Owner

No, the currently last not merged branch only contains activation of scaled contour symbols for the Cairo backend. It does though contain some refactoring o the SymbolRenderer.

I'm just waiting for the build to finish...to merge it.

@vyskocil
Copy link
Collaborator Author

Ok thanks, I'll try it again when all is merged but as I understand it, it should not change much.
Actually I'm not using the SymbolRenderer in the iOS/OSX backend, I only updated the previous DrawSymbol code in the iOS driver adding code to append a affine transform before drawing to scale the symbol accordingly but if it receive 0 it would not work :-)
Then I think there is something not working when osmscout-map call the iOSX driver maybe there is a issue with :

symbolScale=data.mainSlotWidth*pathSymbolStyle->GetScale()/symbolBoundingBox.GetHeight();

Maybe this code use some wrong value from the iOS driver ?

@Framstag
Copy link
Owner

Hmm, symbolBoundingBox.GetHeight() is general symbol code. pathSymbolStyle->GetScale() comes straight from the stylesheet. mainSlotWidth is calculated in CalculateWayPaths() and should not have driver-specific parameters.

Attached an image to show how it would look like...

test

@Framstag
Copy link
Owner

@Framstag
Copy link
Owner

Framstag commented Sep 18, 2022

@vyskocil

Merged

@vyskocil
Copy link
Collaborator Author

Hi @Framstag !
I put some debug logs in MapPainter.cpp and here's what I'm getting for the symbolScale calculation :

symbolScale=0 data.mainSlotWidth=0 pathSymbolStyle->GetScale()=0.5 symbolBoundingBox.GetHeight()=21.5433

data.mainSlotWidth is always 0 in these cases...

@Framstag
Copy link
Owner

mainSlotWidth gets calculated in CalculateWayPaths(). There it iterates through the line styles. The line style with the empty slot is the "main" style. It defines the width of the way, while other, additional line styles (!slot.empty()) are decorating and likely also have an offset to the center of the way (though all this depends on the actual style definition).

Also StyleConfig::GetWayLineStyles() should do some sorting to assure that the style with the empty slot is the first in the list.

It is also important, that further calculations are done based on the mainSlotWidth further down CalculateWayPaths().

The code looks a little fragile because it seems it only works correctly under the assumption that mainSlotWidth is calculated based on the first line style in the list and that this line style has an empty slot. If this is not the case, things can possibly partly break. This should be easy to check with some additional checking code.

Or CalculateLineWith() always returns 0.0 because of strange style sheet (no width given) or strange effects with the projection class.

If you have a style sheet, that is different from the default one, you can send it to me per mail, and I'll check, if it works for me.

It would be good, if we would have a DrawMapIOSX demo application. This way, it would be simple for me to reproduce and analyze the problem, since I still have a Mac Book.

I hope this help you to further debug the information. If you have an idea, how I can easily reproduce the problem on the Mac (without any in deep knowledge of IOSX development and the Apple development environment), give me a hint. I'LL to take a look at it in this case, too.

@vyskocil
Copy link
Collaborator Author

Hi @Framstag !

I tried the iOS renderer using the standard stylesheet provided in the distribution (just a bit tweaked to remove some types not imported in the database I have on hands), I don't get data.mainSlotWidth == 0 anymore and could see the one way icons ! But there are still very few, almost none road names displayed, even for non one way roads...
Here is a screenshot:

IMG_1575C9165ED1-1

@Framstag
Copy link
Owner

So it sounds like there is something triggered, dependent on the actual style sheet. I'll take a look, if I can make the relevant code more robust. We will see if this will help.

@Framstag
Copy link
Owner

Regarding the text label rendering...

Please check, if MapPainter::DrawWayContourLabel() can extract a label. If yes, this should call DrawWayContourLabel() which will create a contour label entry for it using MapPainter::RegisterContourLabel() (which is Renderer specific). It should just call your labelLayouter.

The list of labels as placed by the LabelLayouter is later on rendered by LabelLayouter::DrawLabels(), calling DrawLabel() and/or DrawGlyphs() back on your Renderer.

Please check at the mentioned code places, if there are still street labels to process. Either they get lost somewhere, or font size is too small to be seeen, color is not visible (alpha!) or similar.

@Framstag
Copy link
Owner

@vyskocil : See #1323. Please check, if this changes anything regarding mainSlotWidth. It should either fix the problem or at least print out a warning (if the logger is configured) with the problematic way and its type. This should allow in turn to check the style sheet for an error.

@Karry
Copy link
Collaborator

Karry commented Sep 27, 2022

Hi @Framstag , I found several issues with your changes from last week. It is visible even with Qt backend. I prepared simple bash script:

#!/bin/bash -x

while [ $# -gt 0 ] ; do
  git checkout "$1"
  suffix="$(git log --pretty=format:'%ci %h' | head -1)"
  make -j 8 DrawMapQt
  ./Demos/DrawMapQt \
    --debug \
    --baseMap ~/Maps/world \
    --dpi 200 \
    ~/Maps/europe-france-20220905-004309 \
    ../stylesheets/standard.oss \
    1920 1080 \
    43.61996 6.969865 \
    524288 \
    "DrawMapQt $suffix.png"
  shift
done

And run this script for last 500 commits in master (git log --pretty=format:'%h' | head -500 | xargs -n 1 ../draw-commit.sh).

Starting with ea36250, result is something I am experienced
DrawMapQt 2022-09-05 10:18:29 +0200 ea3625027

Street labels disappear with e3208ea
DrawMapQt 2022-09-17 14:03:13 +0200 e3208ea1c

Restaurant symbols disappears at e1deda2
DrawMapQt 2022-09-17 14:06:53 +0200 e1deda23e

Church symbol is moved from 91991eb
DrawMapQt 2022-09-17 20:42:12 +0200 91991eb11

And finally, oneway arrows are rendered too big (it is expected) but with high density from bcbaa66
DrawMapQt 2022-09-18 09:59:27 +0200 bcbaa660e

Should I create tickets for individual problems?

@Framstag
Copy link
Owner

Framstag commented Sep 27, 2022

Yes, please make tickets. This should help me fix the individual problems.

@Framstag
Copy link
Owner

Given the reproducer infos by @Karry, I was able to reproduce the problem locally. Thanks. I'll take a look.

@vyskocil
Copy link
Collaborator Author

@vyskocil : See #1323. Please check, if this changes anything regarding mainSlotWidth. It should either fix the problem or at least print out a warning (if the logger is configured) with the problematic way and its type. This should allow in turn to check the style sheet for an error.

I rebased #1323 on the latest code and tried it and there was no warning

@vyskocil
Copy link
Collaborator Author

When zooming a lot on small roundabout with the standard stylesheet I got this on iPhone :

IMG_C5FDA5F37117-1

@vyskocil
Copy link
Collaborator Author

@vyskocil : See #1323. Please check, if this changes anything regarding mainSlotWidth. It should either fix the problem or at least print out a warning (if the logger is configured) with the problematic way and its type. This should allow in turn to check the style sheet for an error.

I rebased #1323 on the latest code and tried it and there was no warning

I removed #1323 and now with my own stylesheet the oneway symbol are not drawn anymore but they are still there with the standard stylesheet

@Framstag
Copy link
Owner

So with my path, things work better with the standard stylesheet (overlay arrows appear), though problem is always there with your stylesheet.

Sounds like my fix improves the situation (and I should merge), while your style sheet seems to have a problem?

In this case you should get a warning from the renderer (seee patch):

if (pathData.mainSlotWidth==0.0) {
  log.Warn() << "Line style for way " << way.GetFileOffset() << " of type " << way.GetType()->GetName() << " results in empty mainSlotWidth";
}

Are you sure logging is active? Else: Can you set a break point (either on the condition or the log output) to check?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature For issues and pull request that request or implement a new feature renderer For issues in the content of the rendering engine
Projects
None yet
Development

No branches or pull requests

3 participants