[Bf-committers] [Bf-blender-cvs] SVN commit: /data/svn/bf-blender [52084] trunk/blender/source/gameengine: bge mesh conversion speedup, avoid calling ConvertMaterial() on every face .
Campbell Barton
ideasman42 at gmail.com
Mon Nov 12 01:30:41 CET 2012
Hi Mitchell,
I wasn't aware your patch addressed this issue of "in-efficient
BGE-per-face-material conversion", I have no problems with reverting
my commit to avoid conflicts so your patch can be applied instead (if
its resolving more issues & is stable).
But I think shows some issues in BGE development at the moment.
Developing features in a branch to review is fine (Async LibLoad
sounds like one of these).
But making useful fixes you can better do in trunk, no need for patch
review unless you think there are possible problems you want another
dev to check.
(Of course patch review is generally good - but with a small BGE team
we cant always manage it).
To have a branch and make many changes in different areas, then expect
others to review is not working so well IMHO,
if you make changes in a branch in different areas it takes time for
the reviewer to follow whats a fix, whats an optimization, which
feature some other part of the api was updated for... etc.
>From my perspective - reasonable improvements/fixes should always be
able to be made in trunk, unless we agree to freeze certain areas for
a branch to be merged.
Moving forward -
Some options...
- Revert my change, make a patch from the changes in your branch -
have this reviewed, or if you are sure the patch is stable, commit it
to trunk directly.
- Revert my change, lock game-engine-files relating to your patch in
trunk (svn lock even), then commit your patch within some reasonable
timeframe (before release).
- Leave my code in for release, but make no further changes - can roll
back and apply your patch when its reviewed/ready.
On Sun, Nov 11, 2012 at 8:04 PM, Mitchell Stokes <mogurijin at gmail.com> wrote:
> If you wanted to eliminate per face material conversion, you could have
> also looked at my Swiss patch: https://codereview.appspot.com/6446043/
>
> This is one of the things I worked on. However, now there will be some
> serious conflicts with my patch. I can probably salvage the async lib load
> code, but I'll probably have to rework the multi-uv bug fixes.
>
> On Sat, Nov 10, 2012 at 5:54 PM, Campbell Barton <ideasman42 at gmail.com>wrote:
>
>> Revision: 52084
>>
>> http://projects.blender.org/scm/viewvc.php?view=rev&root=bf-blender&revision=52084
>> Author: campbellbarton
>> Date: 2012-11-11 01:54:30 +0000 (Sun, 11 Nov 2012)
>> Log Message:
>> -----------
>> bge mesh conversion speedup, avoid calling ConvertMaterial() on every face.
>> now do per material bucket.
>>
>> Modified Paths:
>> --------------
>> trunk/blender/source/gameengine/Converter/BL_BlenderDataConversion.cpp
>> trunk/blender/source/gameengine/Ketsji/BL_Material.cpp
>> trunk/blender/source/gameengine/Ketsji/BL_Material.h
>> trunk/blender/source/gameengine/Ketsji/KX_BlenderMaterial.cpp
>> trunk/blender/source/gameengine/Ketsji/KX_BlenderMaterial.h
>>
>> Modified:
>> trunk/blender/source/gameengine/Converter/BL_BlenderDataConversion.cpp
>> ===================================================================
>> --- trunk/blender/source/gameengine/Converter/BL_BlenderDataConversion.cpp
>> 2012-11-11 00:39:08 UTC (rev 52083)
>> +++ trunk/blender/source/gameengine/Converter/BL_BlenderDataConversion.cpp
>> 2012-11-11 01:54:30 UTC (rev 52084)
>> @@ -480,6 +480,45 @@
>> const char *name;
>> } MTF_localLayer;
>>
>> +static void tface_to_uv_bge(const MFace *mface, const MTFace *tface,
>> MT_Point2 uv[4])
>> +{
>> + uv[0].setValue(tface->uv[0]);
>> + uv[1].setValue(tface->uv[1]);
>> + uv[2].setValue(tface->uv[2]);
>> + if (mface->v4) {
>> + uv[3].setValue(tface->uv[3]);
>> + }
>> +}
>> +
>> +static void GetUV(
>> + MFace *mface,
>> + MTFace *tface,
>> + MTF_localLayer *layers,
>> + const int layer_uv[2],
>> + MT_Point2 uv[4],
>> + MT_Point2 uv2[4])
>> +{
>> + bool validface = (tface != NULL);
>> +
>> + uv2[0] = uv2[1] = uv2[2] = uv2[3] = MT_Point2(0.0f, 0.0f);
>> +
>> + /* No material, what to do? let's see what is in the UV and set
>> the material accordingly
>> + * light and visible is always on */
>> + if (layer_uv[0] != -1) {
>> + tface_to_uv_bge(mface, layers[layer_uv[0]].face, uv);
>> + if (layer_uv[1] != -1) {
>> + tface_to_uv_bge(mface, layers[layer_uv[1]].face,
>> uv2);
>> + }
>> + }
>> + else if (validface) {
>> + tface_to_uv_bge(mface, tface, uv);
>> + }
>> + else {
>> + // nothing at all
>> + uv[0] = uv[1] = uv[2] = uv[3] = MT_Point2(0.0f, 0.0f);
>> + }
>> +}
>> +
>> // ------------------------------------
>> static bool ConvertMaterial(
>> BL_Material *material,
>> @@ -489,6 +528,7 @@
>> MFace* mface,
>> MCol* mmcol, /* only for text, use first mcol, weak */
>> MTF_localLayer *layers,
>> + int layer_uv[2],
>> const bool glslmat)
>> {
>> material->Initialize();
>> @@ -500,6 +540,9 @@
>> material->glslmat = (validmat)? glslmat: false;
>> material->materialindex = mface->mat_nr;
>>
>> + /* default value for being unset */
>> + layer_uv[0] = layer_uv[1] = -1;
>> +
>> // --------------------------------
>> if (validmat) {
>> // use lighting?
>> @@ -754,33 +797,19 @@
>> // No material - old default TexFace properties
>> material->ras_mode |= USE_LIGHT;
>> }
>> - MT_Point2 uv[4];
>> - MT_Point2 uv2[4];
>> +
>> const char *uvName = "", *uv2Name = "";
>>
>> -
>> - uv2[0] = uv2[1] = uv2[2] = uv2[3] = MT_Point2(0.0f, 0.0f);
>> -
>> /* No material, what to do? let's see what is in the UV and set
>> the material accordingly
>> * light and visible is always on */
>> if ( validface ) {
>> material->tile = tface->tile;
>> -
>> - uv[0].setValue(tface->uv[0]);
>> - uv[1].setValue(tface->uv[1]);
>> - uv[2].setValue(tface->uv[2]);
>> -
>> - if (mface->v4)
>> - uv[3].setValue(tface->uv[3]);
>> -
>> uvName = tfaceName;
>> }
>> else {
>> // nothing at all
>> material->alphablend = GEMAT_SOLID;
>> material->tile = 0;
>> -
>> - uv[0] = uv[1] = uv[2] = uv[3] = MT_Point2(0.0f, 0.0f);
>> }
>>
>> if (validmat && validface) {
>> @@ -798,49 +827,30 @@
>> }
>>
>> // get uv sets
>> - if (validmat)
>> - {
>> + if (validmat) {
>> bool isFirstSet = true;
>>
>> // only two sets implemented, but any of the eight
>> // sets can make up the two layers
>> - for (int vind = 0; vind<material->num_enabled; vind++)
>> - {
>> + for (int vind = 0; vind<material->num_enabled; vind++) {
>> BL_Mapping &map = material->mapping[vind];
>>
>> - if (map.uvCoName.IsEmpty())
>> + if (map.uvCoName.IsEmpty()) {
>> isFirstSet = false;
>> - else
>> - {
>> - for (int lay=0; lay<MAX_MTFACE; lay++)
>> - {
>> + }
>> + else {
>> + for (int lay=0; lay<MAX_MTFACE; lay++) {
>> MTF_localLayer& layer =
>> layers[lay];
>> if (layer.face == 0) break;
>>
>> - if (strcmp(map.uvCoName.ReadPtr(),
>> layer.name)==0)
>> - {
>> - MT_Point2 uvSet[4];
>> -
>> -
>> uvSet[0].setValue(layer.face->uv[0]);
>> -
>> uvSet[1].setValue(layer.face->uv[1]);
>> -
>> uvSet[2].setValue(layer.face->uv[2]);
>> -
>> - if (mface->v4)
>> -
>> uvSet[3].setValue(layer.face->uv[3]);
>> - else
>> -
>> uvSet[3].setValue(0.0f, 0.0f);
>> -
>> - if (isFirstSet)
>> - {
>> - uv[0] = uvSet[0];
>> uv[1] = uvSet[1];
>> - uv[2] = uvSet[2];
>> uv[3] = uvSet[3];
>> + if (strcmp(map.uvCoName.ReadPtr(),
>> layer.name)==0) {
>> + if (isFirstSet) {
>> + layer_uv[0] = lay;
>> isFirstSet = false;
>> uvName =
>> layer.name;
>> }
>> - else if (strcmp(layer.name,
>> uvName) != 0)
>> - {
>> - uv2[0] = uvSet[0];
>> uv2[1] = uvSet[1];
>> - uv2[2] = uvSet[2];
>> uv2[3] = uvSet[3];
>> + else if (strcmp(layer.name,
>> uvName) != 0) {
>> + layer_uv[1] = lay;
>> map.mapping |=
>> USECUSTOMUV;
>> uv2Name =
>> layer.name;
>> }
>> @@ -853,8 +863,8 @@
>> if (validmat && mmcol) { /* color is only for text */
>> material->m_mcol = *(unsigned int *)mmcol;
>> }
>> - material->SetConversionUV(uvName, uv);
>> - material->SetConversionUV2(uv2Name, uv2);
>> + material->SetUVLayerName(uvName);
>> + material->SetUVLayerName2(uv2Name);
>>
>> if (validmat)
>> material->matname =(mat->id.name);
>> @@ -900,6 +910,7 @@
>>
>> // Extract avaiable layers
>> MTF_localLayer *layers = new MTF_localLayer[MAX_MTFACE];
>> + int layer_uv[2]; /* store uv1, uv2 layers */
>> for (int lay=0; lay<MAX_MTFACE; lay++) {
>> layers[lay].face = 0;
>> layers[lay].name = "";
>> @@ -997,33 +1008,43 @@
>> bool twoside = false;
>>
>> if (converter->GetMaterials()) {
>> + const bool is_bl_mat_new = (bl_mat ==
>> NULL);
>> + //const bool is_kx_blmat_new = (kx_blmat
>> == NULL);
>> const bool glslmat =
>> converter->GetGLSLMaterials();
>> const bool use_mcol = ma ? (ma->mode &
>> MA_VERTEXCOLP || glslmat) : true;
>> /* do Blender Multitexture and Blender
>> GLSL materials */
>> - MT_Point2 uv[4];
>> + MT_Point2 uv_1[4];
>> + MT_Point2 uv_2[4];
>>
>> /* first is the BL_Material */
>> - if (!bl_mat)
>> + if (!bl_mat) {
>> bl_mat = new BL_Material();
>> - ConvertMaterial(bl_mat, ma, tface,
>> tfaceName, mface, mcol,
>> - layers, glslmat);
>> + }
>>
>> - /* vertex colors and uv's were stored in
>> bl_mat temporarily */
>> + /* only */
>> + if (is_bl_mat_new || (bl_mat->material !=
>> ma)) {
>> + ConvertMaterial(bl_mat, ma, tface,
>> tfaceName, mface, mcol,
>> + layers, layer_uv,
>> glslmat);
>> + }
>> +
>> + /* vertex colors and uv's from the faces */
>> GetRGB(use_mcol, mface, mcol, ma, rgb0,
>> rgb1, rgb2, rgb3);
>> + GetUV(mface, tface, layers, layer_uv,
>> uv_1, uv_2);
>>
>> - bl_mat->GetConversionUV(uv);
>> - uv0 = uv[0]; uv1 = uv[1];
>> - uv2 = uv[2]; uv3 = uv[3];
>> + uv0 = uv_1[0]; uv1 = uv_1[1];
>> + uv2 = uv_1[2]; uv3 = uv_1[3];
>>
>> - bl_mat->GetConversionUV2(uv);
>> - uv20 = uv[0]; uv21 = uv[1];
>> - uv22 = uv[2]; uv23 = uv[3];
>> + uv20 = uv_2[0]; uv21 = uv_2[1];
>> + uv22 = uv_2[2]; uv23 = uv_2[3];
>>
>> /* then the KX_BlenderMaterial */
>> if (kx_blmat == NULL)
>> kx_blmat = new
>> KX_BlenderMaterial();
>>
>> - kx_blmat->Initialize(scene, bl_mat,
>> (ma?&ma->game:NULL));
>> + //if (is_kx_blmat_new ||
>> !kx_blmat->IsMaterial(bl_mat)) {
>> + kx_blmat->Initialize(scene,
>> bl_mat, (ma ? &ma->game : NULL));
>> + //}
>> +
>> polymat =
>> static_cast<RAS_IPolyMaterial*>(kx_blmat);
>> }
>> else {
>>
>> Modified: trunk/blender/source/gameengine/Ketsji/BL_Material.cpp
>> ===================================================================
>> --- trunk/blender/source/gameengine/Ketsji/BL_Material.cpp 2012-11-11
>> 00:39:08 UTC (rev 52083)
>> +++ trunk/blender/source/gameengine/Ketsji/BL_Material.cpp 2012-11-11
>> 01:54:30 UTC (rev 52084)
>> @@ -63,11 +63,6 @@
>> share = false;
>>
>> int i;
>> - for (i=0; i<4; i++)
>> - {
>> - uv[i] = MT_Point2(0.f,1.f);
>> - uv2[i] = MT_Point2(0.f, 1.f);
>> - }
>>
>> for (i=0; i<MAXTEX; i++) // :(
>> {
>> @@ -95,40 +90,15 @@
>> }
>> }
>>
>> -void BL_Material::SetConversionUV(const STR_String& name, MT_Point2 *nuv)
>> +void BL_Material::SetUVLayerName(const STR_String& name)
>> {
>> uvName = name;
>> - uv[0] = *nuv++;
>> - uv[1] = *nuv++;
>> - uv[2] = *nuv++;
>> - uv[3] = *nuv;
>> }
>> -
>> -void BL_Material::GetConversionUV(MT_Point2 *nuv)
>> +void BL_Material::SetUVLayerName2(const STR_String& name)
>> {
>> - *nuv++ = uv[0];
>> - *nuv++ = uv[1];
>> - *nuv++ = uv[2];
>> - *nuv = uv[3];
>> -}
>> -void BL_Material::SetConversionUV2(const STR_String& name, MT_Point2 *nuv)
>> -{
>> uv2Name = name;
>> - uv2[0] = *nuv++;
>> - uv2[1] = *nuv++;
>> - uv2[2] = *nuv++;
>> - uv2[3] = *nuv;
>> }
>>
>> -void BL_Material::GetConversionUV2(MT_Point2 *nuv)
>> -{
>> - *nuv++ = uv2[0];
>> - *nuv++ = uv2[1];
>> - *nuv++ = uv2[2];
>> - *nuv = uv2[3];
>> -}
>> -
>> -
>> void BL_Material::SetSharedMaterial(bool v)
>> {
>> if ((v && num_users == -1) || num_users > 1 )
>>
>> Modified: trunk/blender/source/gameengine/Ketsji/BL_Material.h
>> ===================================================================
>> --- trunk/blender/source/gameengine/Ketsji/BL_Material.h 2012-11-11
>> 00:39:08 UTC (rev 52083)
>> +++ trunk/blender/source/gameengine/Ketsji/BL_Material.h 2012-11-11
>> 01:54:30 UTC (rev 52084)
>> @@ -89,18 +89,12 @@
>> EnvMap* cubemap[MAXTEX];
>> unsigned int m_mcol; /* for text color (only) */
>>
>> - MT_Point2 uv[4];
>> - MT_Point2 uv2[4];
>> -
>> STR_String uvName;
>> STR_String uv2Name;
>>
>> - void SetConversionUV(const STR_String& name, MT_Point2 *uv);
>> - void GetConversionUV(MT_Point2 *uv);
>> + void SetUVLayerName(const STR_String &name);
>> + void SetUVLayerName2(const STR_String &name);
>>
>> - void SetConversionUV2(const STR_String& name, MT_Point2 *uv);
>> - void GetConversionUV2(MT_Point2 *uv);
>> -
>> void SetSharedMaterial(bool v);
>> bool IsShared();
>> void SetUsers(int num);
>>
>> Modified: trunk/blender/source/gameengine/Ketsji/KX_BlenderMaterial.cpp
>> ===================================================================
>> --- trunk/blender/source/gameengine/Ketsji/KX_BlenderMaterial.cpp
>> 2012-11-11 00:39:08 UTC (rev 52083)
>> +++ trunk/blender/source/gameengine/Ketsji/KX_BlenderMaterial.cpp
>> 2012-11-11 01:54:30 UTC (rev 52084)
>> @@ -138,7 +138,7 @@
>> RAS_IPolyMaterial::GetMaterialRGBAColor(rgba);
>> }
>>
>> -bool KX_BlenderMaterial::IsMaterial(BL_Material *bl_mat) const
>> +bool KX_BlenderMaterial::IsMaterial(const BL_Material *bl_mat) const
>> {
>> return (mMaterial == bl_mat);
>> }
>>
>> Modified: trunk/blender/source/gameengine/Ketsji/KX_BlenderMaterial.h
>> ===================================================================
>> --- trunk/blender/source/gameengine/Ketsji/KX_BlenderMaterial.h 2012-11-11
>> 00:39:08 UTC (rev 52083)
>> +++ trunk/blender/source/gameengine/Ketsji/KX_BlenderMaterial.h 2012-11-11
>> 01:54:30 UTC (rev 52084)
>> @@ -77,7 +77,7 @@
>> )const;
>>
>> /* mMaterial is private, but need this for conversion */
>> - bool IsMaterial(BL_Material *bl_mat) const;
>> + bool IsMaterial(const BL_Material *bl_mat) const;
>> Material* GetBlenderMaterial() const;
>> MTFace* GetMTFace(void) const;
>> unsigned int* GetMCol(void) const;
>>
>> _______________________________________________
>> Bf-blender-cvs mailing list
>> Bf-blender-cvs at blender.org
>> http://lists.blender.org/mailman/listinfo/bf-blender-cvs
>>
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers
--
- Campbell
More information about the Bf-committers
mailing list