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

Icon Rendering #14

Open
jsvde opened this issue Jul 25, 2017 · 10 comments
Open

Icon Rendering #14

jsvde opened this issue Jul 25, 2017 · 10 comments
Assignees

Comments

@jsvde
Copy link
Contributor

jsvde commented Jul 25, 2017

open questions:

  • what kind of icons should be used? (svg, png,...)
  • dynamic icons? icon font?
@MisterAI MisterAI self-assigned this Aug 16, 2017
@MisterAI
Copy link
Contributor

Repository mit Icons für OSM: https://svn.openstreetmap.org/applications/share/map-icons/svg/

@MisterAI
Copy link
Contributor

Ich hab jetzt mal ein mapping von den meisten verwendeten Icons zu OSM Icons aufgestellt. Scheint auch einigermaßen schnell zu laden. Ich weiß allerdings nicht, wie ich das objektiv mit der Performanz von png Icons vergleichen kann.

@MisterAI
Copy link
Contributor

https://github.com/gravitystorm/openstreetmap-carto/tree/master/symbols
Hier finden sich auf jeden Fall die ganzen OSM Icons, die ich verwendet habe.

@MisterAI
Copy link
Contributor

Im Moment hat das Icon Layer noch eine eigene Source ol.source.IconLabel. Das müssten wohl noch irgendwie mit ol.source.label zusammengeführt werden, um den geclonten Code loszuwerden.

@jgueth
Copy link
Contributor

jgueth commented Aug 28, 2017

Eventuell wäre es auch eine alternative aus ol.source.Label eine Art abstrakte Klasse/Objekt zu machen, die dann ol.source.TextLabel und ol.source.IconLabel erweitern. So könnte man das ganze evtl. ohne Code-Duplizierungen ganz gut abstrahieren und braucht keine Fallunterscheidung in ol.source.Label?!

@MisterAI
Copy link
Contributor

Klingt gut. Dann muss ich mich mal informieren, wie man in JavaScript abstrakte Klassen erstellt. ^^

@MisterAI
Copy link
Contributor

So, ol.source.Label hab ich als "abstrakte Klasse" ausgelagert. Das funktioniert soweit ganz gut. Bei ol.style.Label muss ich nochmal überlegen, wie ich das am besten mache, da dort praktisch nur eine Methode vorhanden ist, die zum Teil gleich bleiben wird. Das hängt aber auch davon ab, ob wir Text+Icon darstellen wollen, oder nur das Icon.

Wir müssen aber auf jeden Fall dann jedes Vorkommen von ol.source.Label in ol.source.TextLabel umbenennen, wenn wir das hier in master mergen.

@MisterAI
Copy link
Contributor

MisterAI commented Oct 5, 2017

@jgueth @jsvde Ich komm hier gerade leider nicht weiter. Ich hab jetzt versucht, auch aus ol.style.Label eine Art abstrakte Klasse zu machen, von der dann ol.style.IconLabel und ol.style.TextLabel erben. Jetzt taucht allerdings das Problem auf, dass this im "Konstruktor" von ol.style.IconLabel und ol.style.TextLabel auf Window gesetzt ist und nicht auf die beiden styles selbst. Dadurch kann ich die protoype-Funktionen im Konstruktor von ol.style.Label nicht aufrufen. Habt ihr eine Idee, wie ich das Problem lösen kann?

@jsvde
Copy link
Contributor Author

jsvde commented Oct 11, 2017

Habe mir das mal angeschaut. Das liegt daran, dass unsere Label-"Klasse" keine Klasse ist sondern eine styleFunction, da ist uns also ein architektonischer Fehler unterlaufen. Du nutzt auch nicht die Inheritance-Funktion die Openlayers für seine Klassen bietet ol.inherits sondern setzt direkt die Konstruktoren/Prototypen. Ich hoffe du bist damit einverstanden, dass ich da ordentlich was umbaue, das tue ich nämlich momentan. Ich baue unter ol/style das Label als wirkliche Klasse auf, so dass ein Label standardmäßig ein TextLabel ist und IconLabel zwar erbt aber das eigentliche Rendering überschreibt. Die styleFunction entscheidet dann nur noch welche Klasse geladen wird.

@MisterAI
Copy link
Contributor

Alles klar. Natürlich hab ich nichts dagegen, wenn du das umbaust. Gibt es da etwas, wobei ich dir helfen kann?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants