Page MenuHome

Cycles/EEVEE: Unify Depth of Field
ClosedPublic

Authored by Jeroen Bakker (jbakker) on May 17 2019, 12:28 PM.

Diff Detail

Repository
rB Blender
Branch
T54659 (branched from master)
Build Status
Buildable 3635
Build 3635: arc lint + arc unit

Event Timeline

Harbormaster completed remote builds in B3624: Diff 15409.

The Alembic stuff looks fine :)

Brecht Van Lommel (brecht) requested changes to this revision.May 17 2019, 12:40 PM

Try running the Cycles regression tests, they are not giving the same result now.

Eevee regression tests can be different since these are all Cycles scenes, but also good to check if those give results matching Cycles.

This revision now requires changes to proceed.May 17 2019, 12:40 PM
source/blender/blenloader/intern/versioning_280.c
3428

Cameara typo

source/blender/blenloader/intern/versioning_cycles.c
429

This can be NULL.

430–433

Nitpick, use 5.6f to avoid implicit cast.

source/blender/makesdna/intern/dna_utils.c
24 ↗(On Diff #15409)

Unnecessary whitespace change?

source/blender/makesdna/intern/makesdna.c
44

Is this needed?

source/blender/makesrna/intern/rna_camera.c
445–446

F-stop -> F-Stop

source/blender/makesrna/intern/rna_camera.c
419

Add rna path definition?

source/blender/makesrna/intern/rna_scene.c
2149–2150

Remove function

Jeroen Bakker (jbakker) marked 5 inline comments as done.

First round of code review. Still need to check to the regressions

source/blender/makesdna/intern/makesdna.c
44

Well I added this as the dna_rename_defs.h is included. Where YF_dofdist is renamed to dof_distance.
as both are deprecated I needed to add it. Is there a better solution for this?

Jeroen Bakker (jbakker) planned changes to this revision.EditedMay 17 2019, 1:00 PM

Cycles regressions + cycles should respect the camera.dof.use_dof flag

source/blender/draw/engines/eevee/eevee_depth_of_field.c
181

Why did you remove this?

Cycles respects camera->dof.use_dof

Jeroen Bakker (jbakker) planned changes to this revision.May 17 2019, 2:44 PM

Cycles regression

Removed the radius aperture_type

Harbormaster completed remote builds in B3635: Diff 15426.
Jeroen Bakker (jbakker) marked an inline comment as done.May 17 2019, 4:55 PM
Brecht Van Lommel (brecht) requested changes to this revision.May 17 2019, 5:03 PM
Brecht Van Lommel (brecht) added inline comments.
source/blender/blenloader/intern/readfile.c
3887

For versioning, you still need to read ca->dof_ob as well.

source/blender/blenloader/intern/versioning_280.c
3443

Set camera->dof_ob = NULL; after this.

This revision now requires changes to proceed.May 17 2019, 5:03 PM

I'll commit with some fixes.

This revision is now accepted and ready to land.May 17 2019, 6:00 PM
This revision was automatically updated to reflect the committed changes.

Removing the radius aperture type is a bit disappointing. I typically model in inch scale and consequently I need vanishingly small f-stops to get any visible dof effect. setting the aperture to .5 works much better and feels like a more intuitive method to me than setting the f-stop to .01