-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Conversation
2.0/AudioSimple/README.md
Outdated
|
||
## Screenshot | ||
|
||
![screenshot](screenshot/screenshot.jpg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the file extensions do not match, you probably need to change the extension of the referenced file to .png
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I think the size of screenshot.png
is bigger than the others. can you change 1200x1200
to about 400x400
?
This is because screenshot.png
is also used for list display.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cx20 On the one hand, I agree. On the other hand, it's a bit subtle:
- One possible reason to use smaller screenshots is to not bloat the size of the repository. This particularly refers to samples where the visual presentation is not important (like this one): There is no visual detail that is relevant in the screenshot. One could argue about that for other models, where the screenshot shows "a nice rendering result"
- The size of the screenshot affects the column in the table in a non-obvious way. Markdown does not explicitly allow setting the column width for a table. And the GitHub markdown preview apparently does some sort of "weighting": The screenshot here is 1200 pixels wide, but the table column is not - it is only "relatively wide" compared to the other columns...
Subjectively, I like such screenshots and their width to be a bit more "regular" and "predictable". For example:
- This is the current "Showcase" table: https://github.com/javagl/glTF-Sample-Models/tree/master/2.0#showcase-1
- This is the "Showcase" table with fixed image widths: https://github.com/javagl/glTF-Sample-Models/tree/table-column-width/2.0#showcase-1
The latter can (unfortunately) only be achieved with some (non-markdown) <img...>
tags. But I think that it looks nicer this way...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(It's a detail, but I just brought this up in https://github.com/KhronosGroup/glTF-Sample-Models/issues/362 nevertheless...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the final decision on this? I fixed the file extension, should I still resize the thumbnail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@robertlong Thank you for correcting the file extension.
I think it would be nice to have a small image for the thumbnail, but I'm not too particular about it.
BTW, Regarding the screenshot, how about making it a forest rather than the sea?
The reason I think so is because the environmental sounds are the chirping of birds.
…Models into KHR_audio-simple
"mesh": 1, | ||
"name": "Sphere", | ||
"scale": [0.5, 0.5, 0.5], | ||
"translation": [5.605831146240234, 1.0160478353500366, 0], | ||
"extensions": { | ||
"KHR_audio": { | ||
"emitter": 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This node has both an audio emitter and a Mesh. This significantly complicates import into Godot Engine which only allows one type per node, since we would need to import 2 nodes in this case.
Instead, I would be in favor of modifying the KHR_audio standard to only allow audio emitters on nodes that aren't meshes (or cameras, lights, physics nodes, etc). Even aside from implementation difficulty, it makes more sense to have these be separate, so that an object's audio transform is not necessarily locked to the mesh's transform.
There is no ratified Audio extension. This PR will be closed without merge by 29 Nov. Future PRs should only use ratified extensions and be against Sample-Assets repo. |
PR closed because there is no ratified extension. A new PR can be opened in glTF-Sample-Assets when an extension reaches 'release candidate' status. |
This PR contains a minimal sample asset for testing the draft KHR_audio extension.
It includes the source model and a glTF + glb version of the asset. The model was generated in Blender and the audio extension was added by hand. The glb was compiled using Third Room's implementation of glTF-transform.