search

furti / FreeCAD-ArchTextures

Add Textures to Architectural Objects in FreeCAD
MIT License
28 stars 13 forks source link

getTextureForMaterial fails with attribute Name not found #36

Open memetb opened 4 years ago

memetb commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 https://github.com/furti/FreeCAD-ArchTextures/blob/e3af6198ea5e07848602a3b8ba01ebab2335d6b1/texture_manager.py#L224 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 commented 4 years ago

@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 commented 4 years ago

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.

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 commented 4 years ago

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 commented 4 years ago

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