dpolivy commented on issue #600:
URL: 
https://github.com/apache/cordova-plugin-camera/issues/600#issuecomment-632256215


   After doing a lot of experimentation, I think there are a few areas for 
improvement in how the plugin manages scaling. In my scenario above, with a 
Pixel 2 at a resolution of 4032 x 3024, to scale down to 620px max is yielding 
a sample size of 6. So Android is only using every 6th pixel of the original 
image, and clearly that's not producing a great result.
   
   When I adjust my maximum dimensions to achieve a sample size of 4 or lower, 
then the scaling result is actually quite good quality.
   
   I understand that the original scaling code here may have been Android best 
practice in the past, however I think it can be improved upon. I found a [great 
overview of scaling options on 
Android](https://stackoverflow.com/a/32121059/1347600) that uses a combination 
of the `inSampleSize`, `inScaled`, `inDensity` and `inTargetDensity` options to 
scale the image directly to the desired size in a single API call. Using this 
approach would eliminate the need to call `Bitmap.createScaledBitmap` again 
after loading the image. Additionally, since this plugin is dealing with at 
most one image at a time (vs loading lots of potentially large Bitmaps at 
once), it seems that maybe it can be a bit more open to using more memory 
upfront to achieve a better end result.
   
   Secondly, I might suggest that we limit the sample size to a maximum of 4, 
which would help save some memory on initial load, but subsequently provide a 
better starting image for the actual scaling process to act upon. There are 
some issues in the way `calculateSampleSize` works: the documentation says:
   
   > Note: the decoder uses a final value based on powers of 2, any other value 
will be rounded down to the nearest power of 2.
   
   The function doesn't adhere to that, and seems to return various values that 
are not powers of 2. In reality, it seems Android itself [may support 
this](https://stackoverflow.com/questions/37596945/why-bitmapfactory-options-insamplesize-doesnt-be-rounded-down-to-the-nearest-po),
 although it may be less efficient for the codec, and/or it could lead to some 
of the quality issues I'm seeing.
   
   I suppose the ultimately resolution here depends on what the goals are of 
the scaling. In my opinion, it should be the highest quality representation 
possible for the desired resolution. If that means slightly higher memory use 
for a short period while the image is loaded and scaled, that might be worth 
it. Heck, even loading the full image and scaling it down properly may use less 
memory by only loading the image once, vs the current approach which loads it 2 
or 3 times (depending also on whether it is to be rotated).
   
   For now, I've worked around this issue by increasing our maximum dimensions 
so the scaling result is better, but it does seem like this could be improved 
in the plugin.
   
   Anyone else have thoughts on the best behavior? I'm open to try and put 
together a PR if there is some interest and discussion.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to