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

Optimizes BlackWhiteRaster. #107

Merged
merged 2 commits into from
Jul 17, 2019
Merged

Optimizes BlackWhiteRaster. #107

merged 2 commits into from
Jul 17, 2019

Conversation

tatarize
Copy link
Contributor

No description provided.

@tatarize
Copy link
Contributor Author

Clean version of #102. WhiteBlackRaster overtly corrects for incongruity with the meaning of "black". Delegates to the cleaner version of the class as part of piecemeal approach.

@tatarize
Copy link
Contributor Author

Trying to use the latest code for the driver without changing things I realized the BlackWhiteRaster needed to be able to return the RasterElement underlying it. The RasterBuilder function is great but requires that class type. And I documented the functions you asked about.

Rebasing this only needs to drop BlackWhiteRaster in the branch to replace BlackWhiteRaster. Since the other class already went through.

@t-oster
Copy link
Owner

t-oster commented Jul 17, 2019

Do we need to test this with all existing drivers or should it be transparent to the outside?

@tatarize
Copy link
Contributor Author

The BlackWhiteRaster had entirely private internal structures. All the functions in here return the same data they returned in the old version. I simply delegated to a different class that does the same things in a more robust manner, thus being correct to replace the GreyRaster and AdapterBufferImage classes. So it should be a modular change.

In addition, BlackWhiteRaster isn't actually directly called by any of the drivers. Those use the helper functions in the RasterPart code. These elements of reading and writing in the object are covered in the test functions for BlackWhiteRaster and RasterPart.

The intent was to replace BlackWhiteRaster with RasterElement but that was too severe, so this merely delegates to the more robust code. Which will speed things a bit, but should categorically not break anything.

@t-oster t-oster merged commit f06f25b into t-oster:master Jul 17, 2019
@tatarize tatarize deleted the tatarize-rasteroptimize branch July 19, 2019 07:38
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.

2 participants