Larger Canvas Size Support
Primary goal is to fix #492 which caused crashes at various "large" resolutions for different people.
- SampleBuffer class to hold the sample buffer and alpha buffer of render as double[][] instead of double[]
- Changed BitmapImage class to use int[][] instead of int[], to be consistent with SampleBuffer
- Made BitmapImage.data protected for better encapsulation.
- Refactor things to work with the new SampleBuffer and BitmapImage. And use the getters and setters instead of directly accessing array (which is faster in most cases too).
- Fixed wrong comment in FloatingPointCompressor class for decompression logic
- PngFileWriter now takes BitmapImage instead of int[]; PfmFileWriter does similar internally.
- BitmapImage function local variables refactored to correct names.
- BitmapImage#vFlipped() now uses System.arraycopy and is thus much faster.
Known Issues:
- Denoiser plugin no longer can directly access double[], and will need to be updated to use the SampleBuffer (and while changing that, may as well switch to the existing PfmFileWriter)
- Limits "Render Preview" canvas size in RenderCanvasFx class to prevent strange JavaFx crashes. This should be changed in the future. The current state can only view the top left corner of the render. I might change this to only show the middle 4k x 4k section, but ideally we should have some zoomed out preview which isn't so big that it crashes (and doesnt take much CPU time away from actual rendering).
As discussed on discord, this is a temporary solution at best, and proper render regions would be the better approach. How much does this PR hurt performance? Some profile results would be nice on multiple setups, I'll get some results on my machine later today. High sample counts will be the most affected most likely
This pull request is marked as a draft pull request, as it has a few issues with efficiency as well as compatibility. In its current state, I do not expect it to be accepted, without some decent improvements.
I believe it has done better than master in most of the test cases due to a couple slight changes in a few of the files that were made, so I will be creating a separate pull request to fix a couple of those things, should they prove to be the cause of increased efficiency.
2.3.0-55-gad38006d0 - 3399s, 312347 SPS
max-res-fix - 421s, 314,944 SPS
2% faster
---
1920x1080 32 SPP
2.3.0-55-gad38006d0 - 435s, 304,495 SPS
max-res-fix - 421s, 314,944 SPS
3.2% faster
---
3840x2160 128 SPP
2.3.0-56-g4419bd7e - 3377s, 314,331 SPS
max-res-fix - 3273s, 324,291 SPS
3% faster
At the very least this Draft does not reduce performance.
@StanleyMines This PR (along with #798) is pretty much exactly what I'd need to add non-uniform sample distribution (canvas cropping, selective rendering, …). The performance seems to be slightly better than before, this PR looks pretty good.
Is there anything (other than the BitmapImage.java conflict) that would need to be changed before merging this?
Un-'draft'ed this PR.
Still need to fix test cases, though I think everything else is working fine atm. (@leMaik knows more about what exactly needs to be done.)
Note: Hard coded 4096x4096 display instead of full image is not sufficient. This still crashes chunky on my other computer, when using my second monitor.
Note: Hard coded 4096x4096 display instead of full image is not sufficient. This still crashes chunky on my other computer, when using my second monitor.
This seems to depend on the windowed size of the chunky GUI itself. When it is large (5000x3000 ish) it crashes at small sizes (2.5k by 2.5k) when window is small (900x300) it can do a bit larger (5kx5k), although I cannot seem to get larger than 5kx5k even on single display, and minimal window size, at least on this computer. (Java 11.0.3 + JavaFX LTS 11.0.2; as well as java 1.8.0u281)
Both the UI and Canvas need access to the same Texture from what I can gather with JavaFX. The maximum supported texture size depends on the GPU, driver, and D3D/OpenGL pipeline used.
-Dprism.verbose=true should give us a quick readout of Maximum supported texture size: x which we could use as the baseline but we would also need to factor the UI size. I'm not sure about a programmatic way we can check the hardware capabilities but at least with OpenGL calling GetIntegerv(GL_MAX_TEXTURE_SIZE) would work.
I've not followed this too closely but what I suggest is to limit the on-screen canvas to have its biggest dimension be in some interval, let's say [1000; 2000] px. You then take the biggest dimension of the real canvas and you divide it by 2 until it falls into the interval. For example, for a (imaginary) 76005100 canvas, you do the division twice to get an on-screen canvas of 19001275 which javafx supports without issue and each on-screen pixel correspond to 4*4 pixels in the real canvas, that's a round number easy to deal with. To get the on-screen canvas from the real canvas can be done in the finalizePixel function so that it only reads 1 sample out of x (4 in this example) and write it to the backBuffer.
@StanleyMines I'd like to drop the changes done to BitmapImage. They are not required for anything but saving the final image (if that image has more than 2^31-5 pixels, which it will not have). I'd like to keep this as simple as possible, which would be:
- [x] Have the
SampleBufferat full resolution and with variable spp - [ ] Store number of samples of every pixel in the dump file
- [ ] Specify area to be rendered before starting the rendering
- [ ] Only display the area that is being rendered, show spp for that area… behave like this area is what is being rendered
- [x] Merge variable-spp dumps (and also merge the rendered area into it automatically after rendering, but that's already what Chunky does)
This would allow people to manually split their images into tiles and merge them, it could also be automated by automatically generating the json files (we'd need some new property like "crop": [x,y,w,h] in it) and would also enable area-based tile splitting in ChunkyCloud.
Once that's done, we can always come back to this and add a nice UI or a view for the entire image. But the most important aspect about this is to be able to split large resolution jobs into multiple smaller jobs, which is not possible at the moment.
Added Downscaleing to render previews:
- Downscaled preview should no longer crash on 99% of computers, and will display entire render.
Fixed Merging:
- wrong spp indices in SampleBuffer methods
- missing spp coefficient for merge samples
- merge tests didn't set spp for whole scene sample buffer
Future things to consider doing:
- We should really have some way to set RenderCanvasFx#previewShouldSubsample, though I haven't implemented that yet.
- We should probably update preview scale %s to reflect the downscaling. (And this should work great with @alexhliu's new auto-scaling preview PR, #810 )
- Preview render should probably also only show up to the max shown size instead of rendering far more pixels than will actually be shown. It's a quick preview after all, is it not?
- (from leMaik's comment) 💡 In the future we could add a
PreviewSampleBufferthat supports not having a sample for every pixel and uses the color of the nearest neighbor sample to create an image. That would solve #736
This should fix the issue described in #492 where a single array cannot hold the whole image, however I just realized that #492 does not mention the issue of strange JavaFx crashes as I mentioned in the first comment of this PR, which is the issue that is discussed by #27 and #314. This JavaFx crash has an annoying part to it that it fails and crashes the JavaFx thread, where there is no chunky code on the stack trace at all, with no outwardly noticeable indication of the crash (other than resizing the window might lead to flickering or red boxes, which is not something that I think most people are constantly doing when waiting for canvas size to increase). Since it is entirely in JavaFx code, there isn't an easy way that we can wrap it in a try-catch and either restart it with a smaller canvas or notify the user with a warning/error dialog.
Working on adding rendering subareas and spp to dump; converting PR back to draft until that works with dumps and merging. (hopefully tonight or tomorrow)
This should complete:
- [x] Store number of samples of every pixel in the dump file
- [x] Specify area to be rendered before starting the rendering
- [x] Only display the area that is being rendered, show spp for that area… behave like this area is what is being rendered
However there are still a few issues:
- namely dump is uncompressed and thus larger,
- and dunno why, but its also many many times slower. (eg 1080p scene is ~10-15 seconds per save/load). I chose to take out compression for this iteration due to having trouble troubleshooting the compressed dumps and thought it might have been a contributor to the slow speed (that might have helped by a bit, but not too sure). Could probably be added back now.
- loading scenes that are cropped via
scene.jsonusually render wrong at first. Something to do with the canvas and double buffering I think, but I can't seem to resolve this. Current fix is to just render one more spp. (Merging doesn't seem to have this issue for some reason) - Scenes that have been cropped or merged from cropped scenes have incorrect Render Time and the SPS calculation is independently broken.
Since it is getting close to ready and most of the new features work, I'll undraft this, but plan on continuing to try to fix these few issues.
The last few commits have been fixes to make it more stable, and fix the problem of extremely slow saving/loading times for dumps. SPS number should also be fixed. SPP is also interleaved with the sample values, and thus merging no longer needs to necessarily read the entire dump before merging the first few pixels. This also means more than two dumps could be merged together, with little to no memory overhead.
Other notes: Dump is still not compressed, so about a 20% size increase over before, if you include spp being added.
Still need work from before:
- loading scenes that are cropped via scene.json usually render wrong at first. Something to do with the canvas and double buffering I think, but I can't seem to resolve this. Current fix is to just render one more spp. (Merging doesn't seem to have this issue for some reason)
Which is a bit finicky, but all the features do work.
For the future:
- Add way to edit cropping values without needing to modify scene.json (NOT planned to be in this PR.)
Updates:
- Merging fixed in more ways than one.
- Load Scene UI has been updated.
Things left to do in this PR:
- [x] All things set out to do have been accomplished. (Let me know if anything was missed)
For the future (probably not this PR):
- Way to subdivide renders within the UI (Currently requires manual editing of the scene json file)
- Allow compression of dumps (possibly changing order of chunks within file for better compression (deflate on SPP values))
@leMaik said:
Dump compression needs to come back [...]
So I guess there is (at least) one more thing to do here...
IntegratedSppDump now uses gzip to compress all of the dump after the header. This is slower than the floating point compressor by a bit, and is far far slower than uncompressed, but is capable of saving ~10% size. Merge testing is currently not functional for the new features though.
Next thing I plan to work on is adding some (jank) UI way to subdivide a scene and possibly move to the next subarea to render. (I'll fix dump merge test once theres a better solution to compression... atm I don't think its worth putting in more effort to get the test to pass (more than implementing the compression), if I know its working right now and its probably going to be changed very soon...)
@StanleyMines Sounds good :clap: Maybe do the UI on a new branch so that we can finally get this feature merged when the compression is ready