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

Texture coordinates #46

Merged
merged 11 commits into from
Mar 22, 2019
Merged

Texture coordinates #46

merged 11 commits into from
Mar 22, 2019

Conversation

mlin865
Copy link
Contributor

@mlin865 mlin865 commented Mar 22, 2019

No description provided.

Copy link
Member

@rchristie rchristie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly fixes to reduce repeated code.

if self._useCrossDerivatives:
eft.setTermNodeParameter(n*4 + 4, 1, ln, Node.VALUE_LABEL_D2_DS1DS2, 2)

assert eft.validate(), 'eftfactory_bicubichermitelinear.createEftFlattenTube: Failed to validate eft'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function name difference in assert.

def createEftOpenTube(self):
'''
Create a basic bicubic hermite linear element template for elements
along boundary where a tube is opened for a flat preparation. Retain node
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Document: opened on xi1 = 1. Could eventually have 6 variants.

d1Texture.append(d1)
d2Texture.append(d2)

nodeIdentifier = nextNodeIdentifier
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change 'next' to 'first' in nextNodeIdentifier, nextElementIdentifier throughout this function as it's clearer.

textureCoordinates.setNodeParameters(cache, -1, Node.VALUE_LABEL_D2_DS1DS2, 1, zero)
textureCoordinates.setNodeParameters(cache, -1, Node.VALUE_LABEL_D2_DS1DS2, 2, zero)
nodeIdentifier = nodeIdentifier + 1
elif (n+1)%(elementsCountAround+1) > 0:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to just 'else'.

textureCoordinates.setNodeParameters(cache, -1, Node.VALUE_LABEL_D2_DS1DS2, 1, zero)
nodeIdentifier = nodeIdentifier + 1

# create texture coordinate elements
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change comment to 'define texture coordinates field over elements
(no new elements are being created)


elementtemplate1 = mesh.createElementtemplate()
elementtemplate1.setElementShapeType(Element.SHAPE_TYPE_CUBE)
elementtemplate1.defineField(textureCoordinates, -1, eftTexture)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eftTexture1

@@ -243,6 +243,107 @@ def generatetubemesh(region,
result = element.setNodesByIdentifier(eft, nodeIdentifiers)
elementIdentifier = elementIdentifier + 1

# Create texture coordinates
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Define texture coordinates field

u = [ 1.0 / elementsCountAround * n1,
1.0 / elementsCountAlong * n2,
1.0 / elementsCountThroughWall * n3]
d1 = [1.0 / elementsCountAround, 0.0, 0.0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d1 and d2 are not functions of n1, n2 or n3, so no need to calculate and store.
Eventually we may have variability around, but I expect we'll be consistent all the way along, or within each segment.
Personally I'd just store the variation with n1 and n3, then define the rest in the node loop.
Only cache coordinates if there is a performance gain from shared calculation.

textureCoordinates.setNodeParameters(cache, -1, Node.VALUE_LABEL_D_DS1, 1, d1Texture[n])
textureCoordinates.setNodeParameters(cache, -1, Node.VALUE_LABEL_D_DS2, 1, d2Texture[n])
endIdx = n + elementsCountAround
textureCoordinates.setNodeParameters(cache, -1, Node.VALUE_LABEL_VALUE, 2, uTexture[endIdx])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the code in the if and else clauses is the same except setting the second version for the last nodes. Put the common code in one place. i.e. at the end:

if n == 0:
    textureCoordinates.setNodeParameters(cache, -1, Node.VALUE_LABEL_VALUE, 2, uTexture[endIdx])
    textureCoordinates.setNodeParameters(cache, -1, Node.VALUE_LABEL_D_DS1, 2, d1Texture[endIdx])
    textureCoordinates.setNodeParameters(cache, -1, Node.VALUE_LABEL_D_DS2, 2, d2Texture[endIdx])
    if useCrossDerivatives:
        textureCoordinates.setNodeParameters(cache, -1, Node.VALUE_LABEL_D2_DS1DS2, 2, zero)


nodeIdentifier = nextNodeIdentifier
for n in range(len(uTexture)):
if n%(elementsCountAround+1) == 0.0:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

== 0, not a floating point.
Also, I like spaces around operators % and +.
That said, I would loop as follows:

for n3 in range(elementsCountThroughWall + 1):
    for n2 in range(elementsCountAlong + 1):
        for n1 in range(elementsCountAround):

... and then compute coordinates from those precalculated around one slice.

Copy link
Member

@rchristie rchristie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good.

@rchristie rchristie merged commit 003b2ac into ABI-Software:master Mar 22, 2019
@mlin865 mlin865 deleted the textureCoord branch March 24, 2019 20:00
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

Successfully merging this pull request may close these issues.

2 participants