Conversation
wheeland
force-pushed
the
save_tangents_ape
branch
from
0686b7e to
c6017e4
Compare
| viewMatrix.row(2)[3]); | ||
| } | ||
|
|
||
| int addToJsonArray(QJsonObject &object, const QString &name, const QJsonObject &toInsert) |
There was a problem hiding this comment.
there are already some "JSON-util" like functions here : https://github.com/KDAB/kuesa/blob/dev/src/core/gltf2exporter/gltf2utils_p.cpp
maybe they should all be put in some kind of generic json_utils.hpp ?
There was a problem hiding this comment.
put it into gltf2utils_p.cpp for now
| return m_context; | ||
| } | ||
|
|
||
| void GLTF2Parser::generateMissingTangents(const QString &basePath, QJsonObject &rootObject) |
There was a problem hiding this comment.
I think that this should be refactored to go in the exporter part - maybe by storing a flag in one of the GLTF2Context struct which indicates which mesh had its tangents computed and then by adding a pass here before the draco compression (which might compress the tangent attribute) : https://github.com/KDAB/kuesa/blob/dev/src/core/gltf2exporter/gltf2exporter_p.cpp#L199 ?
wheeland
force-pushed
the
save_tangents_ape
branch
3 times, most recently
from
dbb2b6c to
13c8807
Compare
|
As of now, I kept this in GLTF2Importer. Doesn't feel like the best place to put it though. Moving everything to GLTF2Exporter doesn't make sense of now, as this one is only created once during export, and we want to see the tangents in action within APE, as materials might depend on them, right? I guess one way to make it cleaner would be to move the tangent computation itself into GLTF2Context for example, retain some information on what has changed, including a new QByteArray for the newly generated buffer(s), and then in GLTF2Exporter use all that saved info to modify the JSON tree. This would require some slightly awkward duplication of information though, which is not needed as of now. Trying to figure out what breaks the Draco export now. Does the DracoExportPass expect all data to be stored inside a single buffer file? |
|
could use some unit tests |
jalbamon
left a comment
jalbamon
left a comment
There was a problem hiding this comment.
Looks OK from a documentation perspective
On windows, the build had included combaseapi.h which defines "interface" to "struct". This conflicted with a variable we used also named "interface".
DaveeFTW
force-pushed
the
save_tangents_ape
branch
from
66f9aa0 to
06049e0
Compare
6 participants
Task-Id: KUE-617