Details

Reviewers

Bastien Montagne (mont29)
Sybren A. Stüvel (sybren)

Commits

rBe7bea3fb6ed0: Assets/IDs: Don't generate previews for object types with no real geometry

Summary

Object types like empties, cameras or lamps will just end up as empty preview
images. We can think about ways to visualize them still, but meanwhile, don't
create such an empty preview.

Diff Detail

Repository

rB Blender

Branch

temp-object-data-preview-check (branched from master)

Build Status

Buildable 12668
Build 12668: arc lint + arc unit

Event Timeline

Julian Eisel (Severin) requested review of this revision.Feb 5 2021, 6:34 PM

Julian Eisel (Severin) created this revision.

Harbormaster completed remote builds in B12666: Diff 33632.Feb 5 2021, 6:34 PM

Arc ignored the base branch I passed, hopefully this works now

Harbormaster completed remote builds in B12668: Diff 33635.Feb 5 2021, 6:40 PM

Hrm, maybe if I pass the actual commit hash?

Harbormaster completed remote builds in B12669: Diff 33636.Feb 5 2021, 6:42 PM

Arc...

Harbormaster completed remote builds in B12670: Diff 33637.Feb 5 2021, 6:46 PM

Maybe after merging in the base branch (which I branched this one off...)

Edit: Finally that did the trick. Makes little sense but well, that's arc...
Sorry for the noise.

Harbormaster completed remote builds in B12671: Diff 33638.Feb 5 2021, 6:50 PM

Bastien Montagne (mont29) accepted this revision.Feb 9 2021, 11:38 AM

This revision is now accepted and ready to land.Feb 9 2021, 11:38 AM

This patch has an approved status yet it was not committed yet. So I'm assuming the other reviewers are blocking. Updating the reviewers list to reflect that. This way the patch show as pending still.

This revision now requires review to proceed.Mar 26 2021, 5:15 PM

Sybren A. Stüvel (sybren) added inline comments.Apr 19 2021, 3:23 PM

source/blender/makesdna/DNA_object_types.h
473	What is the difference between "geometry" and "real geometry"?

Julian Eisel (Severin) added inline comments.May 19 2021, 7:24 PM

source/blender/makesdna/DNA_object_types.h
473	We could just call it `OB_TYPE_HAS_GEOMETRY` or `OB_TYPE_SUPPORTS_GEOMETRY` or `OB_TYPE_IS_GEOMETRY` or ... :) Maybe I'm being pedantic calling it "real" geometry, but thing is that even empties or cameras and such have some geometry for drawing (stored in a GPU batch). But that is of course not what we refer to here.

Julian Eisel (Severin) mentioned this in T82704: Polish Preview Creation.Jul 26 2021, 7:22 PM

Julian Eisel (Severin) added a project: Asset Browser.Jul 29 2021, 2:33 PM

Sybren A. Stüvel (sybren) added inline comments.Oct 25 2021, 11:35 AM

source/blender/makesdna/DNA_object_types.h
473	even empties or cameras and such have some geometry for drawing (stored in a GPU batch). Indeed, I wouldn't refer to that as the object "having geometry". When you throw these two kinds of geometry onto one pile, you also get confusion about what "renderable geometry" means -- does a mesh with hidden faces have "renderable geometry"? Or a mesh object without any vertices? I think that `OB_TYPE_IS_GEOMETRY` is the clearest. Some object types are geometry types, others are not, and IMO that should be unambiguous.

On second thought, maybe it's better to make this "enable/disable preview" switch more explicit, and not just let it be related to having geometry or not. The current previews for worlds can work quite well:

I'd be more happy with those than with previewless ones.

Also Actions don't have geometry, but we do have preview rendering code in place to generate those previews.

This revision now requires changes to proceed.Oct 25 2021, 11:39 AM

In D10334#341924, @Sybren A. Stüvel (sybren) wrote:

On second thought, maybe it's better to make this "enable/disable preview" switch more explicit, and not just let it be related to having geometry or not. The current previews for worlds can work quite well:

I'd be more happy with those than with previewless ones.

Also Actions don't have geometry, but we do have preview rendering code in place to generate those previews.

This patch is only for Object assets, so that objects with non-geometry object-data (cameras, empty, etc.) do not get a preview rendered. So this patch doesn't affect worlds or actions.

:facepalm:

LGTM with the change to the macro and the avoidance of "real" geometry ;-)

This revision is now accepted and ready to land.Oct 25 2021, 11:52 AM

Closed by commit rBe7bea3fb6ed0: Assets/IDs: Don't generate previews for object types with no real geometry (authored by Julian Eisel (Severin)). · Explain WhyOct 25 2021, 1:33 PM

This revision was automatically updated to reflect the committed changes.

Julian Eisel (Severin) added a commit: rBe7bea3fb6ed0: Assets/IDs: Don't generate previews for object types with no real geometry.

Assets/IDs: Don't generate previews for object types with no real geometry
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 33635

source/blender/blenkernel/BKE_asset.h

source/blender/blenkernel/BKE_icons.h

source/blender/blenkernel/BKE_idtype.h

source/blender/blenkernel/intern/action.c

source/blender/blenkernel/intern/asset.c

source/blender/blenkernel/intern/brush.c

source/blender/blenkernel/intern/collection.c

source/blender/blenkernel/intern/icons.cc

source/blender/blenkernel/intern/image.c

source/blender/blenkernel/intern/lib_id.c

source/blender/blenkernel/intern/light.c

source/blender/blenkernel/intern/material.c

source/blender/blenkernel/intern/object.c

source/blender/blenkernel/intern/scene.c

source/blender/blenkernel/intern/screen.c

source/blender/blenkernel/intern/texture.c

source/blender/blenkernel/intern/world.c

source/blender/blenloader/intern/readblenentry.c

source/blender/editors/util/ed_util_ops.c

source/blender/makesdna/DNA_object_types.h

Assets/IDs: Don't generate previews for object types with no real geometryClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 33635

source/blender/blenkernel/BKE_asset.h

source/blender/blenkernel/BKE_icons.h

source/blender/blenkernel/BKE_idtype.h

source/blender/blenkernel/intern/action.c

source/blender/blenkernel/intern/asset.c

source/blender/blenkernel/intern/brush.c

source/blender/blenkernel/intern/collection.c

source/blender/blenkernel/intern/icons.cc

source/blender/blenkernel/intern/image.c

source/blender/blenkernel/intern/lib_id.c

source/blender/blenkernel/intern/light.c

source/blender/blenkernel/intern/material.c

source/blender/blenkernel/intern/object.c

source/blender/blenkernel/intern/scene.c

source/blender/blenkernel/intern/screen.c

source/blender/blenkernel/intern/texture.c

source/blender/blenkernel/intern/world.c

source/blender/blenloader/intern/readblenentry.c

source/blender/editors/util/ed_util_ops.c

source/blender/makesdna/DNA_object_types.h

Assets/IDs: Don't generate previews for object types with no real geometry
ClosedPublic
Actions

Revision Contents
Changeset List