-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Optimized the images so they look the same, but are less bytes in size #112
Conversation
Typically how long does it take for a full request to be accepted or rejected? I see some of them open for a long time. |
Hi, As for your pull request, the thing that worries me is that optimising images should be a step of the building process. We should not care about keeping files in the development repository optimised, because they come from many sources and it's hard to verify whether they are optimised or not. Therefore, I think that it's not a correct pull request. It does not fix the issue (lack of optimisation step in the building process) but only hide its symptoms. As for the CKBuilder, we planned to add images optimisation, but I can't find a ticket on http://dev.ckeditor.com, so I created ckeditor/ckbuilder#2. |
Okay, it is your code, but I would completely disagree with your assertion that there is any problem with the build process. Images are rarely updated (most were updated a year ago) and the optimization takes a long time to perform, so it would be better to do it at checkin time than every time. What I have seen done is a unit style test that checks all of the images and fails the test if any of them are not optimal. The optimization I ran against this code base's images is set up to remove every last un-needed bytes and took 3 minutes to run. How long should I expect ckeditor to continue using (and for my users to continue downloading the bloated) images before you put in your strategic fix? |
First of all, it's not my code ;). The opinion was mine though and it was even challenged by other members of the CKEditor team. So I decided to do one test in order to understand if at least for some time this patch will improve the situation. I built a CKEditor package using the
I have no idea why the first size is smaller, I was guessing that it will be equal to the second number. But I proved what I wanted to prove - builder reverts all optimisations because it takes all icons files and creates a new file. Clever optimisations are lost, because that's not a compression but some tricks within the PNG format. That's why optimising the dev version does not make sense. Other image files are slightly smaller, but they are not loaded (except may the anchor image) with CKEditor, so that change is negligible and even mitigated by the bigger icons strip.
@wwalc: What do you think? We were planning to rewrite CKBuilder in Node.JS so to avoid using Java. But since we dropped this idea for now because of lack of time, can we speed up the output image files optimisation feature? And can we do that for the online CKBuilder, taking into account the fact that optimisation takes some time, but on the other hand that there will be far less images in the built package than in the whole dev repo? @eperret: BTW. What tool have you used? |
The optimization should be done by builder during the release process because we have no influence on icons used by 3rd party plugins included in custom builds, thus we cannot assume that optimized images are provided. In fact whether the image is optimized at the beginning or not doesn't make any difference as internally images are read into the memory, translated into bitmap and then a new sprite image is created, using its own algorithm. Using different optimization technique will not slow down the buld process significantly. In Java it's already too long, so it doesn't matter while the online builder will be done in a way so that it was as fast as before, when we find time to fix it... |
You could add a soft dependency on http://www.advsys.net/ken/util/pngout.htm, http://pmt.sourceforge.net/pngcrush/, http://advancemame.sourceforge.net/doc-advpng.html and http://optipng.sourceforge.net/. Whenever any of those are available in the environment, you apply them to all images. That way, you only have to:
(https://imageoptim.com/ uses all these PNG tools, and applies them in sequence, because they each have different optimizations — running all of them guarantees the smallest possible size.) |
Performance is a key here. We need to think about smart caching in case of custom builds, because otherwise we'll kill our servers. Or better - use a library which saves PNGs that are already optimised. |
No description provided.