Page MenuHome

Cycles Bake: show progress bar during bake
ClosedPublic

Authored by Dalai Felinto (dfelinto) on Jul 17 2014, 4:55 PM.

Details

Summary

Developer note: I changed where the sample loop is, this way we can update the progress bar every time a sample is finished and in the future even update the result buffer for baking preview.

Diff Detail

Repository
rB Blender

Event Timeline

Please read my new comments. Also I did not test it with CUDA (no nvidia here) but it should be working.

intern/cycles/blender/blender_session.cpp
558

This is actually not needed, the default update function includes some redraw routine that is not required, but I think it's fine to use it instead of the update_status_progress. So consider this gone in the final patch, unless someone has any consideration on the matter.

intern/cycles/render/bake.cpp
164

My main concern with this approach (and one I would like to hear from other devs) is if we are adding too much overhead by dispatching a task job per sample.

An alternative is to treat the individual tasks as tiles, so when they increment sample it's counted against the overall samples (num_samples * num tasks). That said I think this approach is better in the future when we want to have preview of the baking result back to Blender. For baking it's better to show the preview of the entire image per sample instead of per parts I believe (though again we may need a threshold to not update things every sample).

Thoughts?

I'm really unsure about most of the changes you made here. Moving samples cycle from device to session would likely lead to less effective cache utilization. If you'll dispatch individual samples of tiles as separate tasks you'll have even less effective cache usage and will introduce quite reasonable amount of overhead.

Also i have no idea why do you need all those changes and not just probably separate progress_update callback. Cycles is totally capable of reporting progress when using single tile (which as i understood is the main reason why you started to bother with all those changes).

Sergey Sharybin (sergey) requested changes to this revision.Jul 18 2014, 7:49 AM

@Sergey Sharybin (sergey) what cache? I don't think there is any cache going on for the shader tasks (aka baking).

If you'll dispatch individual samples of tiles as separate tasks you'll have even less effective cache usage and will introduce quite reasonable amount of overhead.

But there are and won't be no tiles, those changes are exclusive to shader tasks.

Also i have no idea why do you need all those changes and not just probably separate progress_update callback.

You mean a 'progress_update' routine that takes into account the specificities of shader baking? As I mentioned above, even if we could mimic the tiles code I still think we can benefit from baking per samples (for an upcoming feature 'preview update').

I will start drafting the 'preview update' routine to give me a better picture of that, but I believe there will be more overhead with preview for baking than we have for rendering, thus it may be better to preview update only once per sample (instead of per tile).

I'm talking about CPU cache.

But there are and won't be no tiles, those changes are exclusive to shader tasks.

I don't really see they're exclusive for shader tasks actually. Moving stuff out from the device also affects rendering. Also not sure why shader tasks are so unique that requires special handling?

What's the "preview update" for baking anyway? Just updating of the image editor when new sample arrives? Bet it's doable the same way as viewport.

Dalai Felinto (dfelinto) updated this revision to Unknown Object (????).Jul 18 2014, 10:18 PM

Another attempt, @Sergey Sharybin (sergey) what do you think?
I don't know how to get the number of tasks for GPU (cuda/opencl)
I don't even have a CUDA at hand so I'm actually curious to see what this code is printing (there is a debug printing that goes from 0.0 to 1.0 - for CPU, for CUDA it may go from 0.0 to 9999.0]

Dalai Felinto (dfelinto) edited edge metadata.

Yet another take (+ rebase from upstream/master)
This one should be cleaner, it works fine in CPU and it should be working in GPU as well.

Somewhat like the patch much better now. Some minor feedback.

intern/cycles/device/device.h
125

Naming suggestion: get_split_task_count()

intern/cycles/device/device_cuda.cpp
768

Suggestion: Make DeviceTask::update_prgoress to use a pointer to render tile insteado f a reference and call it from here with NULL tile. This way you wouldn't worry about calling update to often.

intern/cycles/device/device_task.h
55

Same as above. Or maybe get_subtask_count.

Dalai Felinto (dfelinto) edited edge metadata.

Renames based on Sergey's suggestion, and changes to update_progress based on his suggestion as well

I like the update_progress change, specially because it allows in the future for baking to use the same framework as rendering when it comes to update (assuming we find a way to do so).

Sergey Sharybin (sergey) edited edge metadata.

Apart from some unfinished debug code (by the looks of it) LGTM.

source/blender/editors/object/object_bake_api.c
143 ↗(On Diff #2217)

I'd suggest dropping from the current patch.

This revision is now accepted and ready to land.Jul 25 2014, 4:01 PM
Dalai Felinto (dfelinto) edited edge metadata.
  • Revert "tmp2"
  • Revert "tmp"
  • Merge remote-tracking branch 'upstream/master' into bake-progress-noadd