Based on Brecht Van Lommel's suggestion:
"The best solution would be to sync all view cameras once, then free
Blender memory and then render both views in Cycles."
Details
Details
- Reviewers
Brecht Van Lommel (brecht) Sergey Sharybin (sergey) - Maniphest Tasks
- T58142: blender Crash when use Cycles to render Stereoscopy
- Commits
- rBS699b49e8e9d3: Fix T58142: Crash when use Cycles to render stereoscopy
rBSfa1d5586a1ad: Fix Freestyle (ASAN detected) crash
rB699b49e8e9d3: Fix T58142: Crash when use Cycles to render stereoscopy
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
There is something fishy is going on. If this indeed works, needs quite some explanation. If it doesn't work, needs fixing.
Also, if it is a BlenderSession::render which is supposed to own camera, i think the call to device_free is missing. Otherwise device memory will still be occupied if persistent data option is enabled?
| intern/cycles/blender/blender_session.cpp | ||
|---|---|---|
| 452–453 | Missing spaces? | |
| 479–485 | I'm not sure how this works. Scene's camera is never re-allocated, so you're storing same pointer every time? And later one we'll have a double-free because scene still owns the pointer? | |
| 503 | Comment is a sentence, starting with a capital and ending with a full stop. | |
| 522–523 | Maybe i'm missing something, but i don't think delete works for vector. | |
Comment Actions
You are right. I must have done something wrong in my tests because it is not working here now (windows, though I tested on Linux yesterday). And the logic is indeed wrong.
I need blender_session to own the camera, and pass it to scene->camera. It didn't work in my first try, but I will explore this idea further.
Comment Actions
I updated the patch here but to be honest this won't do the work. We still pass camera override on sync_data(), so we can't just rely on scene->camera/scene->dicing_camera for passing the correct stereoscopy data.
Comment Actions
Talked with @Sergey Sharybin (sergey) , what he proposes is to replace the single RENDER call/api with a more granular one:
I think that could be more generic for any engine
BEGIN_NEW_VIEW_LAYER
And engine can free whatever it likes.
RENDER_NEW_VIEW
And engine updates what's needed for that.
This was we can prepare the depsgraph before every view, free them right before the render, and prepare a new one for the next view and so on and on.
@Brecht Van Lommel (brecht) thoughts?
Comment Actions
- Move the view render loop outside the render engine
Entirely new approach, move the view loop outside the render engine.
We were doing this already for the render layers, it should be fine
to do the same for the views.
Of course potentially we could optimize things, but if we need to
free depsgraph inbetween the views renders, we should create and
desotry them entirely befor each view.
Comment Actions
I don't think this latest update is the right solution.
From what I understand @Sergey Sharybin (sergey) proposed to give view layer / depsgraph switching control to the renderer, which I'm fine with. That solution is still not ideal for this particular problem in my opinion, but giving the renderer more flexibility is fine.
However I don't think taking current camera view control out of the render engine's hands is a good idea.
Rather I think Cycles should to support multiview natively in the future, and store multiple cameras in the scene. For dicing and culling we really should have consistent results between all views, and for that to work we need to take export all cameras first and then do data sync once.
Comment Actions
I rather we just don't do the Blender memory freeing in case of multi-view to fix the bug, and then I can add support for multiple cameras on the Cycles side myself.
Comment Actions
I rather we just don't do the Blender memory freeing in case of multi-view to fix the bug, and then I can add support for multiple cameras on the Cycles side myself.
As a quick solution -- sure. But this all becomes quite a mess, with some decisions made by the pipeline, some decisions made by render engine. There should be a way to make communication with render pipeline more streamlined, without weird decisions made somewhere midway (which are currently related on avoiding data duplication in memory).
Not saying that's an exclusive thing to do, the way how Cycles deals with multi-view might need adoption no matter what we decide from pipeline side.