Should .csg format store camera data?

[thehans]

There is an issue with a test named csgpngtest_logo_and_text. This test is not part of the default test suite, so is not tested by CI servers, but can be tested manually with ctest -C All.

The csgpngtest test type basically exports to .csg, then starts another instance of OpenSCAD importing the csg, and exports an image from that. Finally this is compared with the expected image for cgalpngtest, which comes from a normal CGAL render to PNG.

The problem this causes for logo_and_text.scad is that it has $vp* variables set in the .scad file to define the camera position, that means if you generate expected image from cgalpngtest_logo_and_test, then imported .csg version fails.

Alternatively if one were to update the expected results based on csgpngtest_logo_and_text, then the cgalpngtest will fail.

If its not something anyone cares very strongly about, then the quick fix for this would be to add csgpngtest_logo_and_text to the list of disabled tests in tests/CMakeLists.txt

openscad/tests/CMakeLists.txt

Lines 741 to 807 in 2d27703

	disable_tests(
	# These don't output anything
	dxfpngtest_text-empty-tests
	dxfpngtest_nothing-decimal-comma-separated
	dxfpngtest_nullspace-2d
	svgpngtest_text-empty-tests
	svgpngtest_nothing-decimal-comma-separated
	svgpngtest_nullspace-2d
	# Not useful
	throwntogethertest_internal-cavity
	throwntogethertest_internal-cavity-polyhedron
	throwntogethertest_nullspace-difference
	# these take too long, for little relative gain in testing
	stlpngtest_iteration
	offpngtest_iteration
	stlpngtest_fractal
	offpngtest_fractal
	stlpngtest_logo_and_text
	offpngtest_logo_and_text
	# z-fighting different on different machines
	throwntogethertest_issue1803
	opencsgtest_issue1165
	opencsgtest_issue1215
	throwntogethertest_issue1089
	throwntogethertest_issue1215
	# FIXME: This test illustrates a weakness in child() combined with modifiers.
	# Reenable it when this is improved
	opencsgtest_child-background
	# These tests only makes sense in OpenCSG mode
	cgalpngtest_child-background
	cgalpngtest_highlight-and-background-modifier
	cgalpngtest_highlight-modifier2
	cgalpngtest_background-modifier2
	cgalpngtest_testcolornames
	csgpngtest_child-background
	csgpngtest_highlight-and-background-modifier
	csgpngtest_highlight-modifier2
	csgpngtest_background-modifier2
	csgpngtest_testcolornames
	throwntogethertest_testcolornames
	# This test won't render anything meaningful in throwntogether mode
	throwntogethertest_minkowski3-erosion
	# The inf/nan tests fail when exporting CSG and rendering that output again
	# as currently inf/nan is written directly to the CSG file (e.g. r = inf)
	# which is not valid or even misleading in case a variable inf exists.
	# FIXME: define export behavior for inf/nan when exporting CSG files
	# These tests return error code 1.
	# FIXME: We should have a way of running these and verify the return code
	csgpngtest_primitive-inf-tests
	csgpngtest_transform-nan-inf-tests
	csgpngtest_primitive-inf-tests
	csgpngtest_transform-nan-inf-tests
	# Triggers a floating point accuracy issue causing loaded .csg to
	# render slightly differently
	cgalpngtest_nothing-decimal-comma-separated
	cgalpngtest_import-empty-tests
	cgalpngtest_empty-shape-tests
	csgpngtest_issue1258
	)

[rcolyer]

Well, $fn makes it in (in place at built-in calls), and $t makes it in (evaluated numerically). I then contemplated whether we should consider csg simply what is needed to describe a model, or if it is also designated to describe images to be produced and the contents of preview. The evidence that it is supposed to describe images and preview content is that color is included and also convexity arguments are carried along, none of which have any impact on render results. Therefore, csg output is already designed to reproduce things that only apply to the preview display and to producing an image output, except that apparently we do not carry along the camera information.

Therefore I would say, yes, it seems appropriate to me that camera data should also be a part of this content.

I suspect it was omitted because we are not carrying any other top-level variable assignments along, as they are all evaluated. But I conducted a test and added camera information back into a csg output file, then loaded it, and it was processed correctly utilizing the camera information set at the top. So if we dump it in there, we can already reload it just fine.

I think I'm inclined to say that specific camera data variables should be added to the csg output if and only if they have a manually specified value in the input scad file.

[jordanbrown0]

I would say that we include color not because we want to support preview, but rather because color is part of the definition of the model. The fact that it's only supported in preview is not a "frill" of preview, but rather a missing feature of render.

Note that we do not record places where you set $fn and its friends. Rather, we record places where they are consumed. $fn =10; cube(); will not record $fn in the CSG tree.

[jordanbrown0]

And circle() will record $fn and its friends whether or not you set them.