search

ladybug-tools / ladybug-legacy

:beetle: Ladybug is an environmental plugin for Grasshopper.
http://ladybug.tools
Other
192 stars 82 forks source link

Select surfaces by orientation #417

Closed ayezioro closed 6 years ago

ayezioro commented 6 years ago

Hi, As per this discussion and as a need i have now for some project, i wrote a component that allows to select facade faces (vertical for now) according to their orientation (including tolerance angles) and roofs (only horizontal for now) I believe it can be useful for others, but just want to ask your opinion. This is an example file (including Mostapha's from the link above).

I know that there is HB_separateZonesByOrientation component but it requires further information (HB_Zones) that for LB analysis are not required. If you think it can help i'll commit it to the github. I suppose it can be at tab 5 (Extra). The example has 2 versions. One in Rhinoscript and the second a try to convert it to Rhinocommon. The last is not finished and i'll appreciate a hand to convert the missing commands (just a few ... :-)).

Thanks, -A.

chriswmackey commented 6 years ago

@ayezioro ,

This is definitely helpful and I have found myself in similar situations. However, my initial thought is that there's already a Honeybee component that does something very similar but it's kinda misplaced (it might be better in Ladybug) and it doesn't do exactly what you need. It's the Honeybee_Spearate by Normal component here: image

I think that it may make the most sense to have your work integrated into this component and we move it to the Ladybug Extra tab as you say. This way, we have a component that can deal with any type of surface orientation.

Let me know your thoughts.

ayezioro commented 6 years ago

Hi @chriswmackey, Makes sense to have both components under one "roof". Tough, i like my version better ... :-) Actually, i use the HB component in the same file that i use mine, but for different purposes. Didn't think of unifying them. Funny. What kind of outputs do you think will be better to have. The HB gives all surfaces, just separated by normal. Mine gives only what you asked for. I see cons and pros for both. So what do you think? I'll start working on the merge. Thanks, -A.

ayezioro commented 6 years ago

Hi @chriswmackey, Do you mind checking the above file again? The component to check is the one at the top-right (the only one active). It is supposed to have the same functionality of the HB_separateByNormal + those i implemented. From the _orientation input you can choose the desired main orientation or roof/floor geometry. For the later it will pick all roofs or floors according to the maxUp/Down inputs. I believe it is working fine for release.

What i want to do further (in the future): Pick roofs/floors/walls according to their orientation (as in walls) and inclination ranges.

For the functionality in the example i prefer to have the geometry as a list input (instead of item). What do you think? The file has the 2 options.

As for the name of the component, what do you think? Keep the separateByNormal or FacadeSelector or ...? Thanks, -A.

chriswmackey commented 6 years ago

@ayezioro ,

I apologize that it took so long for me to check this. It's working great on my machine and I can already see that your improvements are going to give this component a new lease on life!

As you say, I think it's pretty much ready to be integrated in a pull request and there are only a few minor edits I would make before you do so:

1) Yes to the list access. It makes the component run faster and someone could always graft the input if they needed the hierarchy of the original poly-surface in the output.

2) "Facade Selector" seems to be a bit of misnomer at this point if it also includes roofs and floors. So I might suggest keeping the "Separate by Normal" name but I really like your icon for the component.

3) It would be really useful to have an input for _north on this component since it's rare to have a model in my office where the model's north matches true north. I leave the decision up to you about whether you want to send a pull request first and we can add this later or you add it in before the pull request.

Thanks again for adding this capability in. I already see that I'm going to use it on some projects soon. -Chris

ayezioro commented 6 years ago

Hi @chriswmackey, I also apologize for this late response (attacked by a virus). Agree with all your suggestions. regarding the icon i can't get the credit. I took it from a file you sent me a while ago with "all" LB icons (back then). Agree the icon is nice. Also agree about the north suggestion, but if it is fine by you i'll take care of this later on. Right now don't have the time to deal with it. So i'll commit the component soon.

You can't believe the cases i've found that the component failed to catch. They were taken care and i think it is working pretty well now. Thanks for the help, -A.

chriswmackey commented 6 years ago

@ayezioro ,

I'm sorry to hear about the virus and I hope that you're feeling better now.

I thought I had seen that icon somewhere before...

Thanks for cleaning up the component and I agree that we can take care of the North input later. If you send a pull request, I'll merge it in.

Thanks again and we're happy you're back from the clutches of the virus! -Chris

ayezioro commented 6 years ago

Thanks for the health wishes @chriswmackey. Almost completely recovered. Sent the pull request. I'll take care about the North input. -A.

chriswmackey commented 6 years ago

Component merged. Thanks @ayezioro !

ek542 commented 5 years ago

Hi everyone, I am trying to use the Ladybug separate by normal component to separate faces of my (large) mesh based on the orientation of the normals. The component description says that meshes can be inputs but when I pass my mesh through this component, I get the following errors:

  1. Solution exception:'NoneType' object has no attribute 'Faces'
  2. Type conversion failed from Mesh to Brep Can someone please help me with this? Thanks, Eesha