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

Fully split oslquery and oslnoise libraries from oslexec #1346

Merged
merged 1 commit into from
Mar 12, 2021

Conversation

lgritz
Copy link
Collaborator

@lgritz lgritz commented Mar 11, 2021

liboslquery and liboslnoise exist as separate libraries for apps that
only need to query shader parameters or use OSL's noise functions,
respectively, but dont want to pull in the rest of OSL and all its
heavy dependencies (including LLVM). But liboslexec also needs that
functionality and so separately put the individual modules inside.

This was causing some problems for a tricky use case, and I can't
think of a good reason for it to be done this way now. So in this
patch I remove the redundant modules and just make liboslquery and
liboslnoise into link dependencies of liboslexec. None of this
should affect downstream projects, I don't think; linking against
liboslexec ought to just automatically pull in those other libs.

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

liboslquery and liboslnoise exist as separate libraries for apps that
only need to query shader parameters or use OSL's noise functions,
respectively, but dont want to pull in the rest of OSL and all its
heavy dependencies (including LLVM). But liboslexec also needs that
functionality and so separately put the individual modules inside.

This was causing some problems for a tricky use case, and I can't
think of a good reason for it to be done this way now. So in this
patch I remove the redundant modules and just make liboslquery and
liboslnoise into *link dependencies* of liboslexec. None of this
should affect downstream projects, I don't think; linking against
liboslexec ought to just automatically pull in those other libs.

Signed-off-by: Larry Gritz <[email protected]>
@lgritz
Copy link
Collaborator Author

lgritz commented Mar 12, 2021

@marsupial, does this look like it will be good for you?

@lgritz
Copy link
Collaborator Author

lgritz commented Mar 12, 2021

So, just so everybody understands, the case that made us do this is if you have library A that links liboslexec (it needs a full shading system), and library B that links liboslquery (it just needs to query OSL shader parameters). Let's say that A and B are independently developed and they don't know anything about each other, but application C needs both A and B. Now you have two copies of the oslquery symbols, one coming from liboslexec via A and the other coming from liboslquery via B. Bad, bad.

@marsupial
Copy link
Contributor

Testing it now!

@marsupial
Copy link
Contributor

LGTM!

@lgritz lgritz merged commit 9d07099 into AcademySoftwareFoundation:master Mar 12, 2021
@lgritz lgritz deleted the lg-oslquery branch March 12, 2021 22:08
@lgritz
Copy link
Collaborator Author

lgritz commented Mar 14, 2021

Oops, this actually has some problems. I forgot that we were relying on some OSLQuery methods being defined differently in oslquery and oslexec, and the conflict is causing problems. I'll try to have a correct fix by Monday.

@lgritz
Copy link
Collaborator Author

lgritz commented Mar 15, 2021

2nd round of this fix is in #1348

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