Datagenerator library #200

AvKhokhlov · 2018-09-24T16:33:46Z

No description provided.

achernigin1987 · 2018-09-26T11:17:26Z

BaikalDataGenerator/Source/data_generator_impl.h

+ /// @param lights - lights range
+ /// 'spp' - spp vector
+ /// 'gamma_correction_enabled' - flag to enable/disable gamma correction
+ /// 'start_cam_id' - the number starting from will be named generated samples


Please, actualize options description

achernigin1987 · 2018-09-26T11:18:39Z

BaikalDataGenerator/Source/data_generator_impl.h

+ std::uint32_t num_bounces,
+ unsigned device_idx);
+
+ void AttachLight(LightObject* light);


We don't need this function. it should be done before datagenerator creation in the client code

achernigin1987 · 2018-09-26T11:36:30Z

BaikalDataGenerator/Source/data_generator_impl.cpp

+
+DataGeneratorImpl::DataGeneratorImpl(SceneObject* scene,
+ size_t output_width,
+ size_t output_height,


output_width -> width, output_height -> height

achernigin1987 · 2018-09-26T11:38:43Z

BaikalDataGenerator/Source/data_generator.h

+
+ rpr_camera* cameras;
+ unsigned cameras_num;
+ int cameras_start_output_idx;


int -> unsigned

achernigin1987 · 2018-09-26T11:39:14Z

BaikalDataGenerator/Source/data_generator.h

+ int cameras_start_output_idx;
+
+ rpr_light* lights;
+ unsigned lights_num;


move this section before cameras

achernigin1987 · 2018-09-26T11:40:04Z

BaikalDataGenerator/Source/data_generator.h

 {
-public:
- explicit ConfigLoader(const DGenConfig& config);
+ char const* output_dir;


move it before progress callback

Already done

achernigin1987 · 2018-09-26T11:41:29Z

BaikalDataGenerator/Source/data_generator_impl.cpp

+#include <vector>
+
+
+struct OutputInfo


move it to anonymous namespace

achernigin1987 · 2018-09-26T11:42:58Z

BaikalDataGenerator/Source/data_generator_impl.cpp

+}
+
+void DataGeneratorImpl::GenerateSample(CameraObject* camera,
+ int camera_idx,


int -> unsigned

achernigin1987 · 2018-09-26T11:43:52Z

BaikalDataGenerator/Source/data_generator_impl.cpp

+DataGeneratorImpl::DataGeneratorImpl(SceneObject* scene,
+ size_t output_width,
+ size_t output_height,
+ std::uint32_t num_bounces,


achernigin1987 · 2018-09-26T11:44:34Z

BaikalDataGenerator/Source/data_generator_impl.h

+ const std::filesystem::path& output_dir);
+
+ SceneObject* m_scene = nullptr;
+ std::uint32_t m_width, m_height;


achernigin1987 · 2018-09-26T11:44:43Z

BaikalDataGenerator/Source/data_generator_impl.h

+
+ SceneObject* m_scene = nullptr;
+ std::uint32_t m_width, m_height;
+ std::uint32_t m_num_bounces;


achernigin1987 · 2018-09-26T11:44:55Z

BaikalDataGenerator/Source/data_generator_impl.h

+ bool gamma_correction_enabled) const;
+
+ void GenerateSample(CameraObject* camera,
+ int camera_idx,


achernigin1987 · 2018-09-26T11:47:59Z

BaikalDataGenerator/Source/data_generator_impl.cpp

+ }
+
+ // log render settings
+ auto* render_attribute = doc.NewElement("renderer");


renderer_attribute

achernigin1987 · 2018-09-26T11:49:49Z

BaikalDataGenerator/Source/data_generator_impl.cpp

+ const std::string& scene_name,
+ unsigned cameras_start_idx,
+ unsigned cameras_end_idx,
+ int cameras_index_offset,


cameras_start_idx, cameras_end_idx and cameras_index_offset are not used here. Please, remove them

achernigin1987 · 2018-09-26T11:51:32Z

BaikalDataGenerator/Source/data_generator_impl.cpp

+ doc.SaveFile(bdg_metadata_file_name.string().c_str());
+}
+
+void DataGeneratorImpl::GenerateSample(CameraObject* camera,


GenerateSample -> GenerateCameraData

achernigin1987 · 2018-09-26T11:54:28Z

BaikalDataGenerator/Source/data_generator_impl.cpp

+{
+ assert(m_width);
+ assert(m_height);
+ assert(num_bounces);


useless asserts, because checks are done in the data_generator.cpp

achernigin1987 · 2018-09-26T12:02:20Z

BaikalDataGenerator/Source/data_generator_impl.h

+ int camera_idx,
+ const std::vector<unsigned>& spp,
+ const std::filesystem::path& output_dir,
+ bool gamma_correction_enabled);


output_dir, gamma_correction_enabled and spp should be members of class

achernigin1987 · 2018-09-26T12:02:40Z

BaikalDataGenerator/Source/data_generator_impl.h

+ /// 'start_cam_id' - the number starting from will be named generated samples
+
+ void SaveMetadata(const std::filesystem::path& output_dir,
+ const std::string& scene_name,


output_dir and scene_name should be member of class

achernigin1987 · 2018-09-26T12:05:12Z

BaikalDataGenerator/Source/data_generator_impl.h

+ // 'scene_file' - full path till .obj/.objm or some kind of this files with scene
+ // 'output_width' - width of outputs which will be saved on disk
+ // 'output_height' - height of outputs which will be saved on disk
+ // 'num_bounces' - number of bounces for each ray


Please, actualize options descriptions

They're correct

achernigin1987 · 2018-09-26T12:08:07Z

BaikalDataGenerator/Source/data_generator.cpp

+
+ // Render outputs for every specified SPP at the given
+ // camera position and save them to separate files
+ int camera_idx = params->cameras_start_output_idx + i;


achernigin1987 · 2018-09-26T12:16:02Z

BaikalDataGenerator/Source/data_generator.h

+
+ unsigned gamma_correction;
+
+ void (*progress_callback)(int /* camera_idx */);


Lets change signature to:
void (*progress_callback)(unsigned start_idx, unsigned current_idx, end_idx);

achernigin1987 · 2018-09-26T12:24:28Z

BaikalDataGenerator/Source/object_loader.cpp

+ auto app_metadata_file_name = m_app_config.output_dir;
+ app_metadata_file_name.append("app_metadata.xml");
+
+ auto* root = doc.NewElement("app_metadata");


app_metadata -> metadata

achernigin1987 · 2018-09-26T12:45:18Z

BaikalDataGenerator/Source/object_loader.cpp

+ m_rpr_cameras.push_back(&camera);
+ }
+
+ SaveAppMetadata(range.begin, range.end);


range.end - 1

achernigin1987 · 2018-09-26T14:46:42Z

BaikalDataGenerator/Source/data_generator.cpp

+ params->gamma_correction != 0);
+
+ // Attach given lights to the scene
+ for (size_t i = 0; i < params->lights_num; ++i)


Move it before data_generator creation

achernigin1987 · 2018-09-26T14:47:39Z

BaikalDataGenerator/Source/data_generator.cpp

+ }
+
+ // camera_end_idx is index of the last rendered camera
+ unsigned camera_end_idx = params->cameras_num - 1;


achernigin1987 · 2018-09-26T14:49:50Z

BaikalDataGenerator/Source/data_generator.cpp

+ auto* camera = CameraObject::Cast<CameraObject>(params->cameras[i]);
+ if (camera == nullptr)
+ {
+ return kDataGeneratorBadCameras;


lets check all cameras before data generator creation

achernigin1987 · 2018-09-26T14:55:25Z

BaikalDataGenerator/Source/data_generator_impl.cpp

+ bool gamma_correction_enable)
+ : m_scene(scene),
+ m_scene_name(scene_name),
+ m_width(width), m_height(height),


move m_height on next line

achernigin1987 · 2018-09-26T14:55:49Z

BaikalDataGenerator/Source/data_generator_impl.cpp

+ m_renderer->SetMaxBounces(num_bounces);
+}
+
+void DataGeneratorImpl::AttachLight(LightObject* light)


Remove this function

achernigin1987 · 2018-09-26T15:02:17Z

BaikalDataGenerator/Source/app_config.h

+
+#include <vector>
+
+#define DEFAULT_START_OUTPUT_IDX (-1)


lets use const int

achernigin1987 · 2018-09-26T16:32:52Z

BaikalDataGenerator/Source/data_generator.cpp

+ {
+ params->progress_callback(0,
+ i + 1,
+ params->cameras_num);


wrong indices

achernigin1987 and others added 17 commits August 31, 2018 13:19

1) changed algorithm for splitting cameras 2) metadata.xml updated

39e8bc2

cosmetic

b6c2df2

Add -list_devices and -device datagenerator options, fix help message

0788f81

Write device info into metadata.xml

bc60de8

Data generator API implementation

08946cb

Fix Windows build. Fix camera indexing

4ad2edc

Comment out SetMask call

2ff80c3

Apply review remarks

7104360

Fix build errors

6b8c8e8

Fix potential memory leak

c5bf0b8

Rename library and application

44b553a

Add more parameter checks

6ea2068

Fix potential memory leak

bb49334

Simplify code reading

4af6410

Add more comments

c96c73f

Code review

79697dd

BaikalDataGenerator/Source/app_config.h

742aaf6

achernigin1987 reviewed Sep 26, 2018

View reviewed changes

Small changes in datagenerator

d7c5eeb

achernigin1987 reviewed Sep 26, 2018

View reviewed changes

Code review

62ec7a2

achernigin1987 reviewed Sep 26, 2018

View reviewed changes

Alexander Khokhlov added 3 commits September 26, 2018 19:42

More review

6fed1f5

Added comment to gamma correction flag

e019eeb

Changed comment style

9c91bdc

achernigin1987 approved these changes Sep 26, 2018

View reviewed changes

'name' replaced with 'file' in xml metadata

0ad28cd

yozhijk merged commit 371fb99 into GPUOpen-LibrariesAndSDKs:master Oct 1, 2018


		unsigned gamma_correction;

		void (progress_callback)(int / camera_idx */);

		#include <vector>


		struct OutputInfo

Datagenerator library #200

Datagenerator library #200

Conversation

AvKhokhlov commented Sep 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achernigin1987 Sep 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achernigin1987 Sep 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achernigin1987 Sep 26, 2018 • edited Loading

Choose a reason for hiding this comment

achernigin1987 Sep 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achernigin1987 Sep 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achernigin1987 Sep 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achernigin1987 Sep 26, 2018 •

edited

Loading

achernigin1987 Sep 26, 2018 •

edited

Loading

achernigin1987 Sep 26, 2018 •

edited

Loading

achernigin1987 Sep 26, 2018 •

edited

Loading

achernigin1987 Sep 26, 2018 •

edited

Loading

achernigin1987 Sep 26, 2018 •

edited

Loading