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.
Details
- Reviewers
mutlaqja - Commits
- R321:48491864429c: Reduce memory use when zooming in fitsviewer
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
- Branch
- fitsviewer-zoom-memory (branched from master)
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 25571 Build 25589: arc lint + arc unit
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?
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 | 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)?