Reduce memory use when zooming in fitsviewer
ClosedPublic

Authored by murveit on Apr 18 2020, 6:01 AM.

Details

Summary

Instead of creating a scaled image, let Qt do the work.
This saves half the memory when zooming 200% or 300%.
I also limited the zoom to 300% running out of memory has been
an issue for users. This change was relatively straight-forward
for the image rendering, but there was complexity with the
overlays drawn, espcially those rendering text.

Test Plan

Load an image, zoom in. You should be limited to 300%. You should notice less than
1Gb of memory use from a ~20megapixel camera, whereas this would be over 2.5Gb
previously. Try the various overlays. They should work the same as before (some line
widths may be slightly different, but I tried to match the previous rendering.
Similarly, test focus, align, guide views, zooming in and so forth.

Diff Detail

Repository
R321 KStars
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
murveit created this revision.Apr 18 2020, 6:01 AM
Restricted Application added a project: KDE Edu. · View Herald TranscriptApr 18 2020, 6:01 AM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
murveit requested review of this revision.Apr 18 2020, 6:01 AM

Thanks I will test this, but I don't think it should be limited to 300%. I sometimes use 400% and want to even go more. Not all are restricted by RAM so we can't limit the feature for all. Is there a way to get the system RAM to be able to make decisions?

murveit updated this revision to Diff 80721.Apr 21 2020, 6:01 AM

Adjust the maximum permissible zoom based on the amount of physical memory.

Jasem, as requested, I added several lines in the constructor to see if the amount of physical memory is known, and if so, I created max zoom values based on that. See lines 154-177 in fitsview.cpp. Could have gone further and made it a function of the image W*H*C and physical memory, but honestly I'm not sure that's a good idea. Let me know if you disagree and if so, how much zoom for how much physcial memory and image size. Now, you can zoom up to 600% if you have 16Gb of memory. On my 32Gb MacbookPro, this is now limited by CPU more than memory. Tested on 32Gb Mac/Catalina (where the limit is 600%) and on Raspbian RPi4 4Gb (where the limit is 300%). According to online doc, some systems may not reveal the amount of physical memory. In that case the limit should be 300%.

Thank you Hy this is so much better! I just have one concern now regarding Windows OS compatibility and then it's good to go.

kstars/fitsviewer/fitsview.cpp
158 ↗(On Diff #80721)

Would this work on Windows?

Honestly, I don't know and can't test. However, it shouldn't fail. See line 161 in fitsview.cpp.
If the system call on Windows doesn't support the _SC_PHYS_PAGES request,
it should return -1 according to http://man7.org/linux/man-pages/man3/sysconf.3.html
and then we should get the 300% default.

According to https://www.mkssoftware.com/docs/man3/sysconf.3.asp there is no _SC_PHYS_PAGES_
and they say the -1 return value should be given, so my code should fail back to a 300% limit.

Apparently, there's a MEMORYSTATUS.dwAvailPhys call on windows, https://github.com/eygilbert/egdb_intl/issues/1
but I can't compile nor test windows code.

If you like, would it be possible for you to please test this on Windows?
(e.g. add a printf to see what the variables are and if their product matches the amount of physical memory, or if one is -1)?

mutlaqja accepted this revision.Apr 21 2020, 7:22 AM
This revision is now accepted and ready to land.Apr 21 2020, 7:22 AM
This revision was automatically updated to reflect the committed changes.