Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix some errors affecting the Web editor #65704

Merged
merged 1 commit into from
Sep 13, 2022
Merged

Fix some errors affecting the Web editor #65704

merged 1 commit into from
Sep 13, 2022
+20 −22

Conversation

akien-mga
Copy link
Member

  • Don't warn about minimized/maximized modes not being available.
  • Blender and FBX export both depend on running thirdparty applications, which can't be done (easily at least) for Web and Android editors.
  • Editor theme complained about not being able to retrieve texture data for an icon. It was only used once so instead of flipping at runtime, let's just add a flipped icon.

Part of #65702.

@akien-mga
Fix some errors affecting the Web editor
1513d76 
- Don't warn about minimized/maximized modes not being available.
- Blender and FBX export both depend on running thirdparty applications,
  which can't be done (easily at least) for Web and Android editors.
- Editor theme complained about not being able to retrieve texture data
  for an icon. It was only used once so instead of flipping at runtime,
  let's just add a flipped icon.

Part of godotengine#65702.
@akien-mga akien-mga added this to the 4.0 milestone Sep 12, 2022
@akien-mga akien-mga requested a review from a team September 12, 2022 14:30
@akien-mga akien-mga requested review from a team as code owners September 12, 2022 14:30
akien-mga
akien-mga commented Sep 12, 2022
View reviewed changes
modules/gltf/register_types.cpp
Comment on lines +145 to +149
// Can't (a priori) run external app on these platforms.
GLOBAL_DEF_RST("filesystem/import/blender/enabled.android", false);
GLOBAL_DEF_RST("filesystem/import/blender/enabled.web", false);
GLOBAL_DEF_RST("filesystem/import/fbx/enabled.android", false);
GLOBAL_DEF_RST("filesystem/import/fbx/enabled.web", false);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah btw this won't work due to this: #64100.

But when we fix that issue it should work fine.

@YuriSizov
Copy link
Contributor

  • Editor theme complained about not being able to retrieve texture data for an icon. It was only used once so instead of flipping at runtime, let's just add a flipped icon.

Huh? Why would the icon be null? That means that every icon is null at that moment, which doesn't sound correct...

@akien-mga
Copy link
Member Author

  • Editor theme complained about not being able to retrieve texture data for an icon. It was only used once so instead of flipping at runtime, let's just add a flipped icon.

Huh? Why would the icon be null? That means that every icon is null at that moment, which doesn't sound correct...

Ref<Image> img = p_texture->get_image(); must retrieve data from the GPU, and that's probably not implemented currently in the WebGL renderer. @clayjohn should have a look at this eventually but in this specific occurrence, fetching data from the GPU to convert a single icon seemed a bit overkill so I figured adding a new icon variant is cleaner.

@clayjohn
Copy link
Member

Ref img = p_texture->get_image(); must retrieve data from the GPU, and that's probably not implemented currently in the WebGL renderer.

Thats right, I added it for desktop but didn't add it for web or mobile as I couldn't test on those platforms at that time. I can add it anytime now. But I agree your solution is cleaner anyway

@akien-mga akien-mga merged commit f8ec046 into godotengine:master Sep 13, 2022
@akien-mga akien-mga deleted the web-editor-fix-some-errors branch September 13, 2022 06:32
@akien-mga akien-mga mentioned this pull request Nov 1, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement platform:web topic:editor
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.
3 participants
@akien-mga @YuriSizov @clayjohn