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.

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

Allow material inheritance from groups or instances on default materials #41

Open
kirabo opened this issue Jan 7, 2022 · 14 comments
Open
Assignees

Comments

@kirabo
Copy link

kirabo commented Jan 7, 2022

no link FaceBackMaterial in group?
//Function is
//static Surface^ FromSU(SUFaceRef face, bool includeMeshes, System::Collections::Generic::Dictionary<String^, Material^>^ materials)
SUMaterialRef mback = SU_INVALID;
SUFaceGetBackMaterial(face, &mback); //Error mback is 0x00
SUStringRef mbackNameRef = SU_INVALID;
SUStringCreate(&mbackNameRef);
SUMaterialGetName(mback, &mbackNameRef);

//test file is
1.zip

@kirabo
Copy link
Author

kirabo commented Jan 7, 2022

testfile is
image

load error no mat info
image

image

@moethu moethu self-assigned this Jan 17, 2022
@moethu moethu added bug and removed enhancement labels Jan 17, 2022
@moethu
Copy link
Owner

moethu commented Jan 17, 2022

I had a look at the model and it looks like your material is applied to the group instead of the surface itself. when selecting the surface both materials are set to the default material. It looks like if the default material is selected, the element can inherit the material from the group level. I'll check if I can retrieve this material setting on group level using SUDrawingElementGetMaterial

@moethu
Copy link
Owner

moethu commented Jan 17, 2022

@kirabo I implemented a Material property on group and on instance level. This property will reflect the material you applied to the group. The surface's material properties will remain empty (or default) because that's what they are set to.

@moethu moethu added enhancement and removed bug labels Jan 17, 2022
@moethu moethu closed this as completed Jan 17, 2022
@kirabo
Copy link
Author

kirabo commented Jan 18, 2022

bug fix
if mat is null,need check parent group mat

ADD TO Surface FromSU

if (string.IsNullOrEmpty(mbackName) && string.IsNullOrEmpty(minnerName)

URef elem;
elem = SketchUpAPI.SUGroupToDrawingElement(group);
if (elem.ptr != IntPtr.Zero)
{
SURef groupMat;
int n = SketchUpAPI.SUDrawingElementGetMaterial(elem, out groupMat);

@moethu
Copy link
Owner

moethu commented Jan 18, 2022

Hi @kirabo I think I get your point. What you are asking for is raising a question of data integrity on my end. The job of this library is to represent the SketchUp Model as closely as possible as it is provided by the C API. Therefore I do not consider this a bug because this is how the data is returned from the C API. Inheriting materials from the group level is an interpretation done by the SketchUp Application. For example if SketchUp would from now on allow a single color only, let's say red, this libraries job would not be to override all materials with a red color. That's up to the application interpreting the data.
Your code is assuming another thing: that the default material would be null. I'd have to check that - but so far the SketchUp C API returns a material with an empty string as a name. If the material were null I should get a null pointer from the API but so far there always is a material.

I can imagine adding this feature as an optional setting when loading the model, so you are aware of the fact that this is more of an interpretation than the data stored in the model. Let me understand one thing: where are you trying to use this? In what context are you using the library? If you are using the Dynamo components I'd rather add this interpretation there.

@moethu moethu changed the title Not Get Mat IN Group Surface???? Allow material inheritance from groups or instances on default materials Jan 18, 2022
@moethu moethu reopened this Jan 18, 2022
@kirabo
Copy link
Author

kirabo commented Jan 18, 2022

import to twinmotion not bug
1.if surface has mat, use surface mat
2.if surface null mat,use parent group mat
3.if parent group mat is null, use parentparent group mat
it is twinmotion logic

my application is unity runtime import skpfile

@moethu
Copy link
Owner

moethu commented Jan 18, 2022

There is one other issue you should be aware of, which are the components. If a component has default materials applied, in sketchup they are overwritten by the instance material. If you have two instances, one with a red material and one with a green material - there is still only one component in the database. Therefore I cannot override the material because it's either red or green. You get my point?

@moethu
Copy link
Owner

moethu commented Jan 19, 2022

@kirabo can you test the above branch and give me feedback please?

@kirabo
Copy link
Author

kirabo commented Jan 20, 2022

yes fix done,thanks 👍

@PauliusVa
Copy link

could someone post a sample how to correctly assign material to surface, as for me it doesnt work.
I added material to skp.material. then also assign it to surface material. but still i get default material instead of my chosen one.

@moethu
Copy link
Owner

moethu commented Apr 15, 2022

@PauliusVa I just checked: applying materials to surfaces isn't implemented yet but I'll add this.

@moethu
Copy link
Owner

moethu commented Apr 15, 2022

@PauliusVa do you want to give this branch a try please: https://github.com/moethu/SketchUpNET/tree/feature/facematerials

@PauliusVa
Copy link

@moethu i have some issues loading assembly file:
if i add this to my reference list: .\SketchUpNET\x64\Debug\SketchUpNET.dll
i have an error: System.IO.FileNotFoundException: 'Could not load file or assembly 'SketchUpNET.dll' or one of its dependencies. The specified module could not be found.'

Seems it is missing another .dll file. any ideas what could be missing ?

@PauliusVa
Copy link

@PauliusVa do you want to give this branch a try please: https://github.com/moethu/SketchUpNET/tree/feature/facematerials

        skp.Materials.Add("black", new Material("black", new Color(100, 120, 120, 50), false, 0.5, true, false, null));
        Surface s = new Surface(OuterEdges, InnerLoops);
        s.FrontMaterial = skp.Materials["black"];
        s.BackMaterial = skp.Materials["black"];
        skp.Surfaces.Add(s);

doesnt seem to work, or am i doing smth wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants