Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Move updateRenderables to clean API #5012

Merged
merged 2 commits into from
May 27, 2016
Merged

Conversation

kkaefer
Copy link
Member

@kkaefer kkaefer commented May 25, 2016

the updateRenderables algorithm currently expects specific maps and collections; We should instead hide this behind an API to simplify testing and expansion.

@kkaefer
Copy link
Member Author

kkaefer commented May 25, 2016

This converts the updateRenderables algorithm to take a few lambdas that perform the respective actions of obtaining a TileData object, creating, retaining and rendering it. In the test suite, we can inject a log recorder into these to verify that the actions are executed in the correct order.

Prerequisite for #123

@kkaefer kkaefer force-pushed the 5012-updaterenderables-api branch 4 times, most recently from 83afb32 to b652331 Compare May 25, 2016 19:51
@kkaefer
Copy link
Member Author

kkaefer commented May 25, 2016

/cc @jfirebaugh

@kkaefer kkaefer force-pushed the 5012-updaterenderables-api branch 4 times, most recently from 8e1e66e to a0c24b6 Compare May 26, 2016 14:20
@jfirebaugh
Copy link
Contributor

Didn't go too deep on this but it seems like a logical path to making it more testable. Should we rename update_renderables_impl.hpp to update_renderables.hpp, since everything includes the impl.hpp anyway?

@kkaefer kkaefer force-pushed the 5012-updaterenderables-api branch from a0c24b6 to b645923 Compare May 27, 2016 08:10
@kkaefer kkaefer force-pushed the 5012-updaterenderables-api branch from b645923 to 1b8d280 Compare May 27, 2016 08:17
@kkaefer kkaefer merged commit 1b8d280 into master May 27, 2016
@kkaefer kkaefer deleted the 5012-updaterenderables-api branch May 27, 2016 08:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants