-
Notifications
You must be signed in to change notification settings - Fork 156
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
Performance improvements #80
Conversation
Depending on the width of the image a lot of int[] have been created, a BitSet with a size of width * height is a better choice here. It uses less memory and is faster than determinating a pixels transparency each time.
Create a copy of the original raster to do the shouldPad test and instant update the raster. This avoids a lot of object creation, hash calculation and garbage collection needed to maintain the HashSet.
It is more efficent to check the boudings outside the loop
should be a little faster
we expect the none transparent pixel to be around the center. we check a 5 * 5 raster first to move faster to the center. after this we still check all pixels to get a complete result.
i found a bug, i will fix it and reopen the pull request |
@joerg1985 nice man! appreciate the work! Glad to see some performance improvement work! It's definitely needed. |
@joerg1985 I know I didn't have any tests before, but if you wouldn't mind adding a couple unit tests, that would also be super helpful for future work on this class. I also don't mind adding them after the fact since I know they were none existent to begin with. :) |
if padding is used there are a lot of transparent lines
I have done some further improvements and added a test for the collision detection. |
this improves the image quality and removes intermediate step
sorry, commited a tmp image
@kennycason I had a look at the unittests but it is hard to write them without good documentation of the classes. I will try add some unittests, but it might take some days. |
the concurrent creation of words could break client code
the graphics object used to create the font metrics had other RenderingHints than the graphics object to write the text, if the default values of the used jvm did not have the TEXT_ANTIALIASING set to TEXT_ANTIALIAS_LCD_HRGB.
@joerg1985 woah! I like your speed ups you noticed. I will very happily try locally! |
…o it the maximum spiral radius might be bigger or smaller than the image width, e.g. if the image height is bigger than the width.
@joerg1985 update on my end. I have recently left my job and will have more free-time to review all this. My goal is we get this in before end of next week. :) |
this happened when the offset was 0
This would be great, i think i am done now with the improvements. I think i fixed issue #60 in this pr, it was because of different rendering hints used by the graphics instances to meassure and to draw. The rtree dependency could now easily removed, to reduce the footprint of the library, this might be good for andriod. The rectangle area of the placed word needs to be masked in the background and the rtree calls need to be removed. |
@joerg1985 I just pulled the branch down to verify locally and I see a couple test failures:
I'm also about to dig into them. I'd love to get this branch merged into the base. i'd love to remove unnecessary dependencies if possible as well |
@joerg1985 I have tested your branch with various integration tests and think it's great. It's definitely faster and the resulting images have a more compact, yet still aesthetically pleasing result. I'm tempted to @ignore the failing test and merge your work in since you've done so much already and then we can address the failed test (text touching the top line) later. I'll await your response for what you'd prefer. |
I added this test, because i was reworking the text rotation, but i disposed this and returned to the original implementation. I think you could ignore the test for now and after the changes have been merged we could have a deeper look in this. I think of might be related to different rendering behaviour of our systems, because the tests did not fail on my Windows 10, x64, openjdk 10.0.2, what ist your setup? |
@joerg1985 Sounds good! I suspected the same thing. I'll merge in, and fix @ignore the test then release. Thanks a lot! and super sorry for the massive delay on merging, I've had a high density of large life events. :) |
Did some improvements to the collision check's and the padding.
This boosted the performance on my machine for CollisionMode.PIXEL_PERFECT.
Kind regards
Jörg