Details

Reviewers

Summary

The ghc::filesystem we wanted to use for a platform compatible replacement of
std::filesystem (see D12197) doesn't behave as we expect or want to. We could
probably find a solution, but it's not worth the extra time. To move this
forward, we can switch the asset catalogs to use Blender's C functions for path
and filesystem handling.

Diff Detail

Repository

rB Blender

Branch

temp-asset-browser-catalogs-c-filestuff (branched from master)

Build Status

Buildable 17156
Build 17156: arc lint + arc unit

Event Timeline

Julian Eisel (Severin) requested review of this revision.Sep 17 2021, 12:51 PM

Julian Eisel (Severin) created this revision.

Harbormaster completed remote builds in B17096: Diff 41983.Sep 17 2021, 12:51 PM

Julian Eisel (Severin) added a project: Asset Browser.Sep 17 2021, 2:54 PM

Julian Eisel (Severin) added inline comments.Sep 17 2021, 3:09 PM

source/blender/blenkernel/intern/asset_catalog.cc
80	We could just use `const char *` only, but I prefer passing around `StringRefs`, even if we have to use `.data()` a lot.
source/blender/blenkernel/intern/asset_catalog_test.cc
50	Note that `BKE_tempdir_session()` explicitly states that it has a trailing slash. So if the path is empty, concatenating with a `/` would result in `/test-temporary-path`. Think the `std::filesystem::path::operator/()` made sure a relative path doesn't become absolute because the parent is empty.

Julian Eisel (Severin) mentioned this in D12197: Dependencies: Drop-in replacement for std::filesystem (in extern/).Sep 17 2021, 3:14 PM

Use StringRefNull to avoid pitfalls with non-terminated strings

The previous code using StringRef was fine logically, but I rather be explicit about what's expected to avoid errors when the code changes.

Harbormaster completed remote builds in B17156: Diff 42096.Sep 20 2021, 5:04 PM

Correct base branch...

Harbormaster completed remote builds in B17157: Diff 42097.Sep 20 2021, 5:05 PM

Partially revert previous update

Thought the used StringRefNull constructor could construct a new,
null-terminated string. Of course that's wrong since it's a non owning type.

Harbormaster completed remote builds in B17161: Diff 42108.Sep 20 2021, 7:32 PM

Julian Eisel (Severin) mentioned this in T91551: Get patch replacing blender::filesystem with C functions merged.Sep 21 2021, 1:35 AM

Update to changes in base branch, use c_str() not data() for std::string

Harbormaster completed remote builds in B17187: Diff 42155.Sep 21 2021, 1:34 PM

Something went wrong with the merge that confused arc, trying again...

Harbormaster completed remote builds in B17188: Diff 42156.Sep 21 2021, 1:40 PM

Sybren A. Stüvel (sybren) accepted this revision.Sep 21 2021, 3:54 PM

This revision is now accepted and ready to land.Sep 21 2021, 3:54 PM

This is merged into the temp-asset-browser-catalogs branch now, which will be merged into master soon.

Think I wanted to close this earlier :)

Commit	Tree	Parents	Author	Summary	Date
5bf246ce4a5c	7058cb1ecf95	7d5f8baf694a	Julian Eisel	Use StringRefNull to avoid pitfalls with non-terminated strings	Sep 20 2021, 5:03 PM
7d5f8baf694a	14779ce5ea83	f37e3a6365e4	Julian Eisel	Use existing C-APIs for path and filesystem handling (Show More…)	Sep 17 2021, 12:50 PM
f37e3a6365e4	431b540133d4	28a76c56b43e a00507c482e2	Julian Eisel	Merge branch 'master' into temp-asset-browser-catalogs	Sep 10 2021, 11:45 AM
28a76c56b43e	6232492c2add	865fe33544a0 45c44a5b5b62	Julian Eisel	Merge branch 'master' into temp-asset-browser-catalogs	Sep 9 2021, 5:49 PM
865fe33544a0	f4f4e3ea7fe9	f08aeaba95f4	Julian Eisel	Fix compile error of asset library test on Windows (Show More…)	Sep 6 2021, 5:54 PM
f08aeaba95f4	eea71578a82b	d0b68f9bb229	Julian Eisel	Correct name of catalog-tree item child container alias (Show More…)	Sep 6 2021, 3:44 PM
d0b68f9bb229	67db2efb66dd	e8eb5d8a7c60	Julian Eisel	Avoid unnecessary allocations in unit test	Sep 6 2021, 3:44 PM
e8eb5d8a7c60	415ca97ad663	ead9a15c1575	Julian Eisel	Add tests for catalog tree (Show More…)	Sep 2 2021, 3:23 PM
ead9a15c1575	84e8eff467cb	d76863c3cc7c	Julian Eisel	Simplify catalog tree creation using std::map (Show More…)	Sep 2 2021, 12:26 PM
d76863c3cc7c	47f11fa0746c	0cf37baad6bf f62eb8ac16c1	Julian Eisel	Merge branch 'master' into temp-asset-browser-catalogs	Sep 1 2021, 7:41 PM
0cf37baad6bf	92cb21b5eb8a	c0c605df3473	Julian Eisel	Initial tree structure to load catalogs into (Show More…)	Sep 1 2021, 7:36 PM
c0c605df3473	b66380f0f868	8c81ff56ae1e	Julian Eisel	Use `ghc::filesystem` as platform compatible replacement for `std::filesystem` (Show More…)	Aug 31 2021, 4:55 PM
8c81ff56ae1e	4e3f2e80e82a	1a20b3d42de1 5d1fa2e67a93	Julian Eisel	Merge branch 'temp-cpp-ghc-filesystem' into temp-asset-browser-catalogs	Aug 31 2021, 4:51 PM
5d1fa2e67a93	5ec67b743f4f	37943b00f2cf	Julian Eisel	Dependencies: Drop-in replacement for std::filesystem (in extern/) (Show More…)	Aug 31 2021, 4:49 PM
1a20b3d42de1	d531bd48140d	ab98982dddb6 37943b00f2cf	Julian Eisel	Merge branch 'master' into temp-asset-browser-catalogs	Aug 31 2021, 4:29 PM
ab98982dddb6	fbd1a3820dbc	ca70b802ffff f8dd0080a9c4	Jacques Lucke	Merge branch 'master' into temp-asset-browser-catalogs	Aug 17 2021, 11:22 AM
ca70b802ffff	bc09d26889cc	0a7e49f20d98	Sybren A. Stüvel	Asset Catalogs: change default catalog definition file name (Show More…)	Aug 11 2021, 3:52 PM
0a7e49f20d98	5c61ba181e0a	6214763cfdbb	Sybren A. Stüvel	Cleanup: use same parameter name in declaration & definition	Aug 11 2021, 3:36 PM
6214763cfdbb	d106c0794c6c	ed1fd8686fd8	Sybren A. Stüvel	Asset Catalogs: clean up catalog paths before using them (Show More…)	Aug 11 2021, 2:48 PM
ed1fd8686fd8	795afd217955	7083e8cf3386	Sybren A. Stüvel	Asset Catalogs: allow adding new catalogs (Show More…)	Aug 11 2021, 1:59 PM
7083e8cf3386	9fbbb4d48c15	09316c8e8a34	Sybren A. Stüvel	Asset Catalogs: gracefully handle catalog IDs without path (Show More…)	Aug 11 2021, 1:35 PM
09316c8e8a34	01483e3163af	9655cbf605dd	Sybren A. Stüvel	Cleanup: asset catalogs, use doxygen comments (Show More…)	Aug 11 2021, 12:21 PM
9655cbf605dd	eae841b7a0cb	cd2ea51c0edf	Sybren A. Stüvel	Asset Catalogs: pass root dir of asset library to AssetCatalogService (Show More…)	Aug 11 2021, 12:21 PM
cd2ea51c0edf	1bf41f6520a3	4cd8cfe87b7a	Sybren A. Stüvel	Cleanup, Asset Catalogs: turn default catalog filename into a constant (Show More…)	Aug 11 2021, 11:35 AM
4cd8cfe87b7a	d108ade3ec04	ec23aceeb99d	Sybren A. Stüvel	Cleanup: Asset Catalogs, make path separator static class member (Show More…)	Aug 11 2021, 11:33 AM
ec23aceeb99d	beb6f20a6c57	76e3efe4c15e 6aae14027828	Sybren A. Stüvel	Merge remote-tracking branch 'origin/master' into temp-asset-browser-catalogs	Aug 11 2021, 11:25 AM
76e3efe4c15e	77d3aac1c716	fc5f3b70ec26	Sybren A. Stüvel	Cleanup: Assets, add asset catalog documentation comments	Aug 9 2021, 5:53 PM
fc5f3b70ec26	621dea232c69	4978e3d8c73c	Sybren A. Stüvel	Cleanup: Assets, remove unused forward/namespace declarations	Aug 9 2021, 5:53 PM
4978e3d8c73c	12ef7d189eb9	3c0cc8195500	Sybren A. Stüvel	Cleanup: Asset Catalogs, make fields private and add `_` suffix.	Aug 9 2021, 5:48 PM
3c0cc8195500	f59fab062527	784b97e280c1	Sybren A. Stüvel	Partially revert "Cleanup: Assets, move AssetLibrary struct to global namespace" (Show More…)	Aug 9 2021, 5:20 PM
784b97e280c1	9ea2e6ec0a76	f45339d05e4d	Sybren A. Stüvel	Cleanup: Assets, move AssetLibrary struct to global namespace (Show More…)	Aug 9 2021, 5:01 PM
f45339d05e4d	5bd3529dc7f0	5526dbeab7ae	Sybren A. Stüvel	Fix pointer ownership issue in `AssetCatalogService::parse_catalog_file()`, and… (Show More…)	Aug 9 2021, 3:28 PM
5526dbeab7ae	f54af5352d1a	624ca21c60a6	Sybren A. Stüvel	Cleanup: C++ to C comment style (Show More…)	Aug 9 2021, 3:09 PM
624ca21c60a6	06d8257a6fe4	82ffcc3602cb	Sybren A. Stüvel	Filelist: load AssetLibrary (and thus catalogs) (Show More…)	Aug 9 2021, 2:13 PM
82ffcc3602cb	55c80ac68164	68ae900553ae 22ed1c7b61d5	Sybren A. Stüvel	Merge remote-tracking branch 'origin/master' into temp-asset-browser-catalogs	Aug 9 2021, 12:39 PM
68ae900553ae	acd83bbc95d4	697f8fc6e0e2 0986992dbd0d	Sybren A. Stüvel	Merge remote-tracking branch 'origin/master' into temp-asset-browser-catalogs	Aug 9 2021, 11:32 AM
697f8fc6e0e2	6578f24e4c22	cb0fe170df65 b417fb92513c	Sybren A. Stüvel	Merge remote-tracking branch 'origin/master' into temp-asset-browser-catalogs	Aug 9 2021, 9:40 AM
cb0fe170df65	65f4045c699c	c120b254e7a7	Sybren A. Stüvel	Assets: add AssetLibrary struct (Show More…)	Aug 6 2021, 5:11 PM
c120b254e7a7	b78cea4435b6	05d988d5c3cb 9cff9f9f5df0	Sybren A. Stüvel	Merge remote-tracking branch 'origin/master' into temp-asset-browser-catalogs	Aug 6 2021, 3:30 PM
05d988d5c3cb	650e18d40cc2	26fead1faf5c b0df8f53ba62	Sybren A. Stüvel	Merge remote-tracking branch 'origin/master' into temp-asset-browser-catalogs	Aug 6 2021, 12:58 PM
26fead1faf5c	88e35567d432	fc5cb4529913 b1a607ce04fe	Sybren A. Stüvel	Merge remote-tracking branch 'origin/master' into temp-asset-browser-catalogs	Aug 3 2021, 11:46 AM
fc5cb4529913	42f7849dac01	57b9f6c52277	Sybren A. Stüvel	Asset Catalogs: add catalog ID to AssetMetaData (Show More…)	Aug 3 2021, 10:22 AM
57b9f6c52277	2b81e1ebba79	bff716113f71	Sybren A. Stüvel	Asset Catalogs: test space in asset catalog path (Show More…)	Aug 2 2021, 12:59 PM
bff716113f71	5ffe94155a60	92dd780b7383	Sybren A. Stüvel	Cleanup: use same parameter name in declaration & definition (Show More…)	Aug 2 2021, 12:58 PM
92dd780b7383	6cf54203c055	28b2d8367877	Sybren A. Stüvel	Cleanup: Asset Catalogs, use default constructors	Aug 2 2021, 12:43 PM
28b2d8367877	d63723f212db	308d26f68c5b aa6041636136	Sybren A. Stüvel	Merge remote-tracking branch 'origin/master' into temp-asset-browser-catalogs	Aug 2 2021, 12:35 PM
308d26f68c5b	e15645f615e6	2f05bfd348a0	Sybren A. Stüvel	Asset Catalogs: rudimentary writing catalog definition files to disk (Show More…)	Jul 30 2021, 1:15 PM
2f05bfd348a0	ccea35c29115	b98735ec296f	Sybren A. Stüvel	Asset Catalogs: first implementation of loading a catalog definition file	Jul 26 2021, 12:05 PM

Use existing C-APIs for path and filesystem handling
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 42096

extern/ghc_filesystem/README.blender

extern/ghc_filesystem/filesystem.hpp

release/scripts/startup/bl_ui/space_filebrowser.py

source/blender/blenkernel/BKE_asset.h

source/blender/blenkernel/BKE_asset_catalog.hh

source/blender/blenkernel/BKE_asset_library.h

source/blender/blenkernel/BKE_asset_library.hh

source/blender/blenkernel/CMakeLists.txt

source/blender/blenkernel/intern/asset.cc

source/blender/blenkernel/intern/asset_catalog.cc

source/blender/blenkernel/intern/asset_catalog_test.cc

source/blender/blenkernel/intern/asset_library.cc

source/blender/blenkernel/intern/asset_library_test.cc

source/blender/blenkernel/intern/asset_test.cc

source/blender/blenlib/BLI_filesystem.hh

source/blender/blenlib/CMakeLists.txt

source/blender/editors/space_file/CMakeLists.txt

source/blender/editors/space_file/filelist.c

source/blender/makesdna/DNA_asset_types.h

source/blender/makesrna/intern/rna_asset.c

Use existing C-APIs for path and filesystem handlingClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 42096

extern/ghc_filesystem/README.blender

extern/ghc_filesystem/filesystem.hpp

release/scripts/startup/bl_ui/space_filebrowser.py

source/blender/blenkernel/BKE_asset.h

source/blender/blenkernel/BKE_asset_catalog.hh

source/blender/blenkernel/BKE_asset_library.h

source/blender/blenkernel/BKE_asset_library.hh

source/blender/blenkernel/CMakeLists.txt

source/blender/blenkernel/intern/asset.cc

source/blender/blenkernel/intern/asset_catalog.cc

source/blender/blenkernel/intern/asset_catalog_test.cc

source/blender/blenkernel/intern/asset_library.cc

source/blender/blenkernel/intern/asset_library_test.cc

source/blender/blenkernel/intern/asset_test.cc

source/blender/blenlib/BLI_filesystem.hh

source/blender/blenlib/CMakeLists.txt

source/blender/editors/space_file/CMakeLists.txt

source/blender/editors/space_file/filelist.c

source/blender/makesdna/DNA_asset_types.h

source/blender/makesrna/intern/rna_asset.c

Use existing C-APIs for path and filesystem handling
ClosedPublic
Actions

Revision Contents
Changeset List