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

More clang-format (this time, testrender and liboslnoise) #1511

Merged
merged 1 commit into from
May 26, 2022

Conversation

lgritz
Copy link
Collaborator

@lgritz lgritz commented May 22, 2022

Signed-off-by: Larry Gritz [email protected]

@lgritz
Copy link
Collaborator Author

lgritz commented May 25, 2022

any objections?

Copy link
Contributor

@sfriedmapixar sfriedmapixar left a comment

Choose a reason for hiding this comment

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

Overall looks good, I marked a few places that could benefit from
//clang-format off
//clang-format on
protections.

bench(" vhashnoise(v)",
[&]() { DoNotOptimize(vhashnoise<const Vec3&>(vval)); });
bench(" vhashnoise(v,f)",
[&]() { DoNotOptimize(vhashnoise<const Vec3&, float>(vval, fval)); });
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This was more readable prior to clang-format.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixing

{ -1.0f, 1.0f, 1.0f, 0.0f },
{ -1.0f, 1.0f, -1.0f, 0.0f },
{ -1.0f, -1.0f, 1.0f, 0.0f },
{ -1.0f, -1.0f, -1.0f, 0.0f } };

Copy link
Contributor

Choose a reason for hiding this comment

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

This was more readable prior to clang-format.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixing

k1 = 0;
i2 = 1;
j2 = 1;
k2 = 0; /* Y X Z order */
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This was more readable prior to clang-format.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixing

sphere_intersect = m_optix_ctx->createProgramFromPTXString(sphere_ptx,
"intersect");
quad_intersect = m_optix_ctx->createProgramFromPTXString(quad_ptx,
"intersect");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This was more readable prior to clang-format.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

meh, gonna leave

Vec3 N, U;
float xalpha, yalpha, eta;
int refract;
};

Copy link
Contributor

Choose a reason for hiding this comment

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

This was more readable prior to clang-format.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is all about to be rewritten anyway

{ CLOSURE_VECTOR_PARAM(RefractionParams, N),
CLOSURE_FLOAT_PARAM(RefractionParams, eta),
CLOSURE_FINISH_PARAM(RefractionParams) } },
{ "transparent", TRANSPARENT_ID, { CLOSURE_FINISH_PARAM(EmptyParams) } },
// mark end of the array
Copy link
Contributor

Choose a reason for hiding this comment

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

This was more readable prior to clang-format.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not going to bother reverting because this is on the verge of being rewritten anyway as we synchronize all the closures with MaterialX

}
break;
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This was more readable prior to clang-format.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

about to get rewritten anyway

m_attr_getters[ustring("camera:shutter_open")]
= &SimpleRaytracer::get_camera_shutter_open;
m_attr_getters[ustring("camera:shutter_close")]
= &SimpleRaytracer::get_camera_shutter_close;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This was more readable prior to clang-format.

<< OIIO::Strutil::timeintervalformat(runtime, 4) << "\n";
std::cout << "Write : "
<< OIIO::Strutil::timeintervalformat(writetime, 4)
<< "\n";
std::cout << "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

This was more readable prior to clang-format.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This will be rewritten anyway as I switch all this stream output to the new format style, so will leave for now.

@lgritz lgritz force-pushed the lg-clang-format branch from b8cfc2d to 2ca8fd7 Compare May 26, 2022 04:19
@lgritz
Copy link
Collaborator Author

lgritz commented May 26, 2022

Thanks for looking over, @sfriedmapixar

I fixed a few where I thought you were definitely right.

I left a few that I know is in sections of code we're going to rewrite shortly anyway (out with the old closures, in with the new).

And I left a few where I thought you were correct that it looked better before, but the practical value of being "better" was minimal. After clang-formatting a few code bases, I've come to accept that a few areas being slightly less beautiful than I could have made it by hand is actually a reasonable price to pay for the benefit of having it all automatic and minimizing the number of places littered with clang-format on/off comments. I do use them to make exceptions, but I try to do it only in the places where it really makes the code significantly more understandable.

@lgritz lgritz merged commit 08706b7 into AcademySoftwareFoundation:main May 26, 2022
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