Skip to content

Conversation

@tschaub
Copy link
Member

@tschaub tschaub commented Dec 28, 2016

Instead of having the map renderers depend on all layer types and all layer renderer types, the layers can take responsibility for creating layer renderers.

This will make it easier to create small builds without having to rely on @defines to indicate dependencies.

@tschaub
Copy link
Member Author

tschaub commented Dec 28, 2016

There is a circular dependency here that is resolved by #6308. The vector tile renderer depends on an enum that is a property of the vector tile layer. And the vector tile layer depends on the vector rile renderer. After #6308, they both can depend on the render type enum without creating a circular dependency.

Copy link
Member

@ahocevar ahocevar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, thanks for taking this on! It is pretty much what I had in mind. Please merge after #6308.

@tschaub
Copy link
Member Author

tschaub commented Dec 28, 2016

Rebasing now that #6308 is in.

@probins
Copy link
Contributor

probins commented Dec 28, 2016

a good step forward, but it makes #6303 more difficult, doesn't it?

@tschaub
Copy link
Member Author

tschaub commented Dec 28, 2016

@probins - we have two significant problems. One is that each of the map renderers depend on layer renderers for all types of layers. The other is that the map depends on both types of map renderer. The suggestion in #6303 would get rid of the ol.ENABLE_WEBGL flag, but we'd be left with the problem of depending on layer renderer types for all layer types. That is, we'd be left with ol.ENABLE_TILE, ol.ENABLE_IMAGE, ol.ENABLE_VECTOR, and ol.ENABLE_VECTOR_TILE.

I opted to get rid of four build flags first. That leaves us with one problem. We can pick up the discussion on #6303 - I'm not sure what the solution will be.

@tschaub tschaub merged commit 7b9690a into openlayers:master Dec 29, 2016
@tschaub tschaub deleted the layer-renderers branch December 29, 2016 02:33
@tschaub tschaub changed the title (WIP) Make layers responsible for creating layer renderers Make layers responsible for creating layer renderers Dec 29, 2016
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

Successfully merging this pull request may close these issues.

3 participants