Set Ready For Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
LGTM, a couple of minor tweaks for readability.
}Late feedback, but maybe a CHECK here that layout_ != kDefault anymore (e.g. that it was resolved), in case the if/else chain above ever gets more complicated. (Mostly just for future documentation purposes).
init->textureType().AsEnum() == V8XRTextureType::Enum::kTextureArray),NIT: Would probably be more readable to just use this enum as the parameter rather than a bool that needs to be commented.
// Cover all shaped layer types.
.then(testCommonXRLayerInitErrors(create_quad_layer, valid_init, t, gl))
.then(testCommonXRLayerInitErrors(create_cylinder_layer, valid_init, t, gl))
.then(testCommonXRLayerInitErrors(create_equirect_layer, valid_init, t, gl))
.then(testCommonXRLayerInitErrors(create_cube_layer, valid_init, t, gl))
// Cube layers don't have texture type specified.
.then(testXRLayerInitTextureArrayErrors(create_quad_layer, valid_init, t, gl))
.then(testXRLayerInitTextureArrayErrors(create_cylinder_layer, valid_init, t, gl))
.then(testXRLayerInitTextureArrayErrors(create_equirect_layer, valid_init, t, gl))
// Cube layers don't have transforms.
.then(testXRLayerInitTransformErrors(create_quad_layer, valid_init, t))
.then(testXRLayerInitTransformErrors(create_cylinder_layer, valid_init, t))
.then(testXRLayerInitTransformErrors(create_equirect_layer, valid_init, t))
// Test layout errors.
.then(testXRLayerInitLayoutErrors(create_quad_layer, valid_init, t, false))
.then(testXRLayerInitLayoutErrors(create_cylinder_layer, valid_init, t, false))
.then(testXRLayerInitLayoutErrors(create_equirect_layer, valid_init, t, false))
.then(testXRLayerInitLayoutErrors(create_cube_layer, valid_init, t, true/*is_cube*/));NIT: I think I'd prefer to group this by layer type for readability. (So you do .then(testCommonXRLayerInitErrors(create_quad_layer, ...))
.then(testXRLayerInitTextureArrayErrors(create_quad_layer, ...))
...
.then(testCommonXRLayerInitErrors(create_cylinder_layer, ...)
...
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Late feedback, but maybe a CHECK here that layout_ != kDefault anymore (e.g. that it was resolved), in case the if/else chain above ever gets more complicated. (Mostly just for future documentation purposes).
Done
init->textureType().AsEnum() == V8XRTextureType::Enum::kTextureArray),NIT: Would probably be more readable to just use this enum as the parameter rather than a bool that needs to be commented.
Done
// Cover all shaped layer types.
.then(testCommonXRLayerInitErrors(create_quad_layer, valid_init, t, gl))
.then(testCommonXRLayerInitErrors(create_cylinder_layer, valid_init, t, gl))
.then(testCommonXRLayerInitErrors(create_equirect_layer, valid_init, t, gl))
.then(testCommonXRLayerInitErrors(create_cube_layer, valid_init, t, gl))
// Cube layers don't have texture type specified.
.then(testXRLayerInitTextureArrayErrors(create_quad_layer, valid_init, t, gl))
.then(testXRLayerInitTextureArrayErrors(create_cylinder_layer, valid_init, t, gl))
.then(testXRLayerInitTextureArrayErrors(create_equirect_layer, valid_init, t, gl))
// Cube layers don't have transforms.
.then(testXRLayerInitTransformErrors(create_quad_layer, valid_init, t))
.then(testXRLayerInitTransformErrors(create_cylinder_layer, valid_init, t))
.then(testXRLayerInitTransformErrors(create_equirect_layer, valid_init, t))
// Test layout errors.
.then(testXRLayerInitLayoutErrors(create_quad_layer, valid_init, t, false))
.then(testXRLayerInitLayoutErrors(create_cylinder_layer, valid_init, t, false))
.then(testXRLayerInitLayoutErrors(create_equirect_layer, valid_init, t, false))
.then(testXRLayerInitLayoutErrors(create_cube_layer, valid_init, t, true/*is_cube*/));NIT: I think I'd prefer to group this by layer type for readability. (So you do .then(testCommonXRLayerInitErrors(create_quad_layer, ...))
.then(testXRLayerInitTextureArrayErrors(create_quad_layer, ...))
...
.then(testCommonXRLayerInitErrors(create_cylinder_layer, ...)
...
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
CHECK(layout_ != V8XRLayerLayout::Enum::kDefault);Ultra NIT: CHECK_NE(layout_, V8XRLayerLayout::Enum::kDefault);
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ultra NIT: CHECK_NE(layout_, V8XRLayerLayout::Enum::kDefault);
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |