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

getTextureForMaterial fails with attribute Name not found #36

Open
memetb opened this issue Feb 11, 2020 · 7 comments
Open

getTextureForMaterial fails with attribute Name not found #36

memetb opened this issue Feb 11, 2020 · 7 comments

Comments

@memetb
Copy link

memetb commented Feb 11, 2020

As the title says, I have a particular project that triggers this problem.

The symptom manifests during the call to textureObjects and results in an exception when enumerating through the objects of the scene. (This is triggered when the "Create a new TextureConfig" button is pressed, and the exception leaves the UI in an unusable state).

If getTextureForForMaterial is made to correctly try/except and return None as it is intended here, this problem goes away.

I'm open to submitting a PR or simply leave it up to you to resolve. On my local system, I've simply wrapped the whole function getTextureForForMaterial in a try block, with a handler as such:

 except Exception: 
        pass

 return (None, None, None)
@furti
Copy link
Owner

furti commented Feb 11, 2020

Hmm catching all exceptions is never a good idea. The fix would be to check hasattr(obj, "Name") before accessing the Name of the given object.

Could you add your FreeCAD Version info to the issue so I can check whats going on?

@memetb
Copy link
Author

memetb commented Feb 12, 2020

ArchTexture commit e3af619.
OS: Windows 10 (10.0)
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.19.19510 (Git)
Build type: Release
Branch: master
Hash: c3eb6d9001c4d60ff7f7cae89f50bd3c965a9940
Python version: 3.7.3
Qt version: 5.12.5
Coin version: 4.0.0a
OCC version: 7.3.0

I should point out that this bug was being triggered by a particular data file I had which was quite complex. When I recreated the same sub-part I wanted to texture in a new project, I did not experience this issue. So whatever the pickled state of those objects is may actually be a remnant of past version of FreeCAD and may stick around essentially forever if one makes use of asset libraries.

I agree that catch-all-pass is a poor idiom. Duck typing the Name attribute only may be sufficient but I'd also warn that line 240 may also have an issue to check out.

One note, though, is that textureObjects being a top-level function, it should have an exception backstop (possibly logged to console), as letting that function raise an exception is putting the UI into an inconsistent and inoperable state (clicking and double clicking on the texture config object does not bring up the task panel, instead only allowing you to rename it).

furti added a commit that referenced this issue Feb 12, 2020
In case, a object has a Material property, and the material has no "Name"
attribute, a exception is raised when texturing it. To prevent this from
happening, we check if the material has a Name. If not, we simply ignore
the object and do not texture it.

fixes #36
@furti
Copy link
Owner

furti commented Feb 12, 2020

@memetb Thanks for the detailed explanation.

  1. I pushed a fix (hopefully as it is not easy to reproduce as you said) for the "Name Not Found" error in this branch: https://github.com/furti/FreeCAD-ArchTextures/tree/no_name_fix. It sounds like you are a bit familiar with coding. So maybe you can check if this commit fixes your problem.

  2. What problem do you see with the code at line 240 in the texture_manager? The texture_cache is always a dict. It is initialized in the constructor. And in

    if imageFile not in self.textureCache:
    it is checked, if the imageFile is inside. When not, it is created. So in line 240 the imageFile should always be there.

  3. textureObjects should not be a top level function. The only time it is called is from the execute method of the TextureConfig. So throwing an exception should not do any harm here. Throwing exceptions from an execute method in FreeCAD should be perfectly fine I think.

@memetb
Copy link
Author

memetb commented Feb 12, 2020

@furti, thanks for the update, and thanks for thinking about the issue as I don't have a lot of spare bandwidth to review the code.

  1. I will check and report back shortly.

  2. I was just flagging this as a possible issue without having done a deeper code review, but looking at your reply, I think that should be fine. As a minor point: is there a good reason not put the texture = self.textureCache[imageFile] assignment (currently line 240) near the texture check (line 224)? This would increased code readability.

  3. I will find a way to reproduce the issue and report back.

@memetb
Copy link
Author

memetb commented Feb 13, 2020

Hi @furti: I went back to check on what's going on when the bug manifests. It seems that the function receives a dictionary at some point.

I started by running

for o in FreeCAD.ActiveDocument.Objects:
    if not hasattr(o, "Name"):
         print(o)

on the culprit project and I get no hits.

Here's the stack trace I get when I click the UI "create a new TextureConfig" button:


Running the Python command 'Create_Config' failed:
Traceback (most recent call last):
  File "C:\Users\memet\AppData\Roaming\FreeCAD\Mod\ArchTextures\create_config.py", line 18, in Activated
    texture_config.createTextureConfig()
  File "C:\Users\memet\AppData\Roaming\FreeCAD\Mod\ArchTextures\texture_config.py", line 307, in createTextureConfig
    textureConfig = TextureConfig(textureConfigObject, fileObject)
  File "C:\Users\memet\AppData\Roaming\FreeCAD\Mod\ArchTextures\texture_config.py", line 219, in __init__
    self.execute(obj)
  File "C:\Users\memet\AppData\Roaming\FreeCAD\Mod\ArchTextures\texture_config.py", line 225, in execute
    self.textureManager.textureObjects()
  File "C:\Users\memet\AppData\Roaming\FreeCAD\Mod\ArchTextures\texture_manager.py", line 97, in textureObjects
    o.Material)
  File "C:\Users\memet\AppData\Roaming\FreeCAD\Mod\ArchTextures\texture_manager.py", line 211, in getTextureForMaterial
    materialName = material.Name

'dict' object has no attribute 'Name'

So it would seem, for some reason a dict is being passed to that function. I actually put a debug print in the code, and it's specifically an empty dict.


edit: alright, I've found a minimal reproducible bug.

  • create new project
  • Enable Part workbench
  • Create 2 new parts (yellow step icon): "Part" and "Part001"
  • Drag Part001 into Part
  • Create a cube
  • Drag cube into Part001
  • switch to Arch Texture
  • Create new TextureConfig

This will cause the above bug to occur (provided you remove that hasattr patch you put in).

Hopefully this'll allow you to figure out why this is happening.

Let me know if you have any other requests.

@szzer
Copy link

szzer commented Nov 7, 2020

Dear Furti,

I've used the arch texture with limited succes, at the times that it works it's great!

But, I get the attribute Name not found most of the times.
I sadly can't tell you the difference between Arch texture working and not working (makes it pretty hard for you to debug I understand).

I'm using FreeCAD 0.19 build:

OS: Windows 10 (10.0)
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.19.22894 (Git)
Build type: Release
Branch: master
Hash: 9eb080488d970d313c538473e7272117ea0a7cd1
Python version: 3.6.8
Qt version: 5.12.1
Coin version: 4.0.0a
OCC version: 7.3.0
Locale: Dutch/Netherlands (nl_NL)

@szzer
Copy link

szzer commented Nov 7, 2020

I have the problem when trying to apply textures to my house design:
https://github.com/szzer/House

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

No branches or pull requests

3 participants