Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
  • Loading branch information
demolke and AThousandShips authored Dec 18, 2024
1 parent 76e17af commit 458e7d0
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 21 deletions.
20 changes: 11 additions & 9 deletions modules/gltf/tests/test_gltf.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,18 @@

namespace TestGltf {

static Node *gltf_import(String file) {
static Node *gltf_import(const String &p_file) {
// Setting up importers.
Ref<ResourceImporterScene> import_scene = memnew(ResourceImporterScene("PackedScene", true));
Ref<ResourceImporterScene> import_scene;
import_scene.instantiate("PackedScene", true);
ResourceFormatImporter::get_singleton()->add_importer(import_scene);
Ref<EditorSceneFormatImporterGLTF> import_gltf;
import_gltf.instantiate();
ResourceImporterScene::add_scene_importer(import_gltf);

// Support processing png files in editor import.
Ref<ResourceImporterTexture> import_texture = memnew(ResourceImporterTexture(true));
Ref<ResourceImporterTexture> import_texture;
import_texture.instantiate(true);
ResourceFormatImporter::get_singleton()->add_importer(import_texture);

// Once editor import convert pngs to ctex, we will need to load it as ctex resource.
Expand Down Expand Up @@ -107,7 +109,7 @@ static Node *gltf_import(String file) {
return p_scene;
}

static Node *gltf_export_then_import(Node *p_root, String test_name) {
static Node *gltf_export_then_import(Node *p_root, const String &p_test_name) {
String tempfile = TestUtils::get_temp_path(test_name);

Ref<GLTFDocument> doc;
Expand All @@ -123,22 +125,22 @@ static Node *gltf_export_then_import(Node *p_root, String test_name) {
return gltf_import(tempfile + ".gltf");
}

void init(const String &p_test, String copy_target = String()) {
void init(const String &p_test, const String &p_copy_target = String()) {
Error err;

// Setup project settings since it's needed for the import process
// Setup project settings since it's needed for the import process.
String project_folder = TestUtils::get_temp_path(p_test.get_file().get_basename());
Ref<DirAccess> da = DirAccess::create(DirAccess::ACCESS_FILESYSTEM);
da->make_dir_recursive(project_folder.path_join(".godot").path_join("imported"));
// Initialize res:// to `project_folder`
// Initialize res:// to `project_folder`.
TestProjectSettingsInternalsAccessor::resource_path() = project_folder;
err = ProjectSettings::get_singleton()->setup(project_folder, String(), true);

if (copy_target.is_empty()) {
return;
}

// Copy all the necessary test data files to the res:// directory
// Copy all the necessary test data files to the res:// directory.
da = DirAccess::create(DirAccess::ACCESS_FILESYSTEM);
String test_data = String("modules/gltf/tests/data/").path_join(p_test);
da = DirAccess::open(test_data);
Expand All @@ -149,7 +151,7 @@ void init(const String &p_test, String copy_target = String()) {
continue;
}
Ref<FileAccess> output = FileAccess::open(copy_target.path_join(item), FileAccess::WRITE, &err);
CHECK_MESSAGE(err == OK, "Unable to open output file");
CHECK_MESSAGE(err == OK, "Unable to open output file.");
output->store_buffer(FileAccess::get_file_as_bytes(test_data.path_join(item)));
output->close();
}
Expand Down
23 changes: 11 additions & 12 deletions modules/gltf/tests/test_gltf_images.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,15 @@
#define TEST_GLTF_IMAGES_H

#include "test_gltf.h"
#include "tests/test_macros.h"

#ifdef TOOLS_ENABLED

#include "editor/editor_file_system.h"
#include "editor/editor_paths.h"
#include "scene/3d/mesh_instance_3d.h"
#include "scene/3d/skeleton_3d.h"
#include "scene/resources/3d/primitive_meshes.h"
#include "scene/resources/image_texture.h"
#include "scene/resources/material.h"

namespace TestGltf {
Ref<Texture2D> _check_texture(Node *node) {
Ref<Texture2D> _check_texture(Node *p_node) {
MeshInstance3D *mesh_instance_3d = Object::cast_to<MeshInstance3D>(node->find_child("mesh_instance_3d", true, true));
Ref<StandardMaterial3D> material = mesh_instance_3d->get_active_material(0);
Ref<Texture2D> texture = material->get_texture(StandardMaterial3D::TextureParam::TEXTURE_ALBEDO);
Expand All @@ -57,7 +52,7 @@ Ref<Texture2D> _check_texture(Node *node) {
for (int x = 0; x < 2; ++x) {
for (int y = 0; y < 2; ++y) {
Color c = texture->get_image()->get_pixel(x, y);
CHECK_MESSAGE(c == Color(x, y, y), "Texture content is incorrect");
CHECK_MESSAGE(c == Color(x, y, y), "Texture content is incorrect.");
}
}
return texture;
Expand All @@ -66,8 +61,10 @@ Ref<Texture2D> _check_texture(Node *node) {
TEST_CASE("[SceneTree][Node] Export GLTF with external texture and import") {
init("gltf_images_external_export_import");
// Setup scene.
Ref<ImageTexture> original_texture = memnew(ImageTexture);
Ref<Image> image = memnew(Image);
Ref<ImageTexture> original_texture;
original_texture.instantiate();
Ref<Image> image;
image.instantiate()
image.instantiate();
image->initialize_data(2, 2, false, Image::FORMAT_RGBA8);
for (int x = 0; x < 2; ++x) {
Expand All @@ -78,11 +75,13 @@ TEST_CASE("[SceneTree][Node] Export GLTF with external texture and import") {

original_texture->set_image(image);

Ref<StandardMaterial3D> original_material = memnew(StandardMaterial3D);
Ref<StandardMaterial3D> original_material;
original_material.instantiate();
original_material->set_texture(StandardMaterial3D::TextureParam::TEXTURE_ALBEDO, original_texture);
original_material->set_name("material");

Ref<PlaneMesh> original_meshdata = memnew(PlaneMesh);
Ref<PlaneMesh> original_meshdata;
original_meshdata.instantiate();
original_meshdata->set_name("planemesh");
original_meshdata->surface_set_material(0, original_material);

Expand Down Expand Up @@ -114,7 +113,7 @@ TEST_CASE("[SceneTree][Node][Editor] Import GLTF from .godot/imported folder wit
Node *loaded = gltf_import("res://.godot/imported/gltf_placed_in_dot_godot_imported.gltf");
Ref<Texture2D> texture = _check_texture(loaded);

// In-editor imports of gltf and texture from .godot/imported folder should end up in res:// if extract_path is defined
// In-editor imports of gltf and texture from .godot/imported folder should end up in res:// if extract_path is defined.
CHECK_MESSAGE(texture->get_path() == "res://gltf_placed_in_dot_godot_imported_material_albedo000.png", "Texture not parsed as resource.");

memdelete(loaded);
Expand Down

0 comments on commit 458e7d0

Please sign in to comment.