Bake-API: Additional required changes
(including function to estimate a width/height when baking into multiple images).
Attempt to Fix T41222 Blender gives weird ouput when baking (4096*4096) resolution on GPU
Details
Details
- Reviewers
Sergey Sharybin (sergey) - Maniphest Tasks
- T41222: Blender gives weird ouput when baking (4096*4096) resolution on GPU
- Commits
- rBS5c3c3abb454c: Cycles Bake: use size_t instead of width, height
rBS9952699978a1: Bake-API: use size_t instead of width, height
rC058fe06b3cc4: Cycles Bake: use size_t instead of width, height
rB5c3c3abb454c: Cycles Bake: use size_t instead of width, height
rB9952699978a1: Bake-API: use size_t instead of width, height
rB1607bcebc65f: Bake-API: use size_t instead of width, height
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
- style cleanup
Updating D688: Bake-API: use size_t instead of width, height
(original patch by Sergey Sharybin)
Comment Actions
LGTM, even if it doesn't solve the issue, i'd say it's something how it should be actually.
Comment Actions
- style cleanup
- update docs
- Merge remote-tracking branch 'upstream/master' into fix-bake-gpu-sizet
Updating D688: Bake-API: use size_t instead of width, height
(original patch by Sergey Sharybin)
Comment Actions
Its worry that there can be multiple images and width/height may be meaningless
| source/blender/editors/object/object_bake_api.c | ||
|---|---|---|
| 181 | Dont think this is correct, since width * height are int's and will overflow before the assignment. Should be (size_t)width * (size_t)height | |
Comment Actions
- prevent size_t overflow - as suggested by Campbell Barton
Updating D688: Bake-API: use size_t instead of width, height
(original patch by Sergey Sharybin)
Comment Actions
@Campbell Barton (campbellbarton) I share your concern, but there is nothing stopping us from calling bake() once per image either. That could even help to make 'preview baking' to work. This is just not how the API was originally envisioned (I may be wrong here, but the point of passing an array of pixels to bake is that you could eventually even send only the UV mapped pixels).
The reason I wanted to bring you into this conversation is that you once told me we should call bake only once with all the objects to bake. And this patch is a move in the other direction (multiple bake calls in oppose to one big bake call).
@Sergey Sharybin (sergey) any final thoughts?
Comment Actions
I talked with @Campbell Barton (campbellbarton) and he said it's doable to allow RNA to handle size_t. Since this doesn't seem to affect the original bug report, I would say we wait for RNA to support size_t and commit only the original changes by @Sergey Sharybin (sergey) (so we don't change the Bake API itself)
Comment Actions
@Dalai Felinto (dfelinto), i'll be fine with adding size_t arguments for functions, adding them to generic API (such as, say. scene props would be much huger change). What about:
- Commit changes to bake_api, which switches it to use size_t
- Also commit changes to Cycles side, but apart from those which touches baking api
- This would make all parts of code which doesn't use RNA API to be ready for the transition
- Work on size_t arguments later
- Once that size_t is supported for RNA functions calls we'll be all good.