RFR: 8271024: Implement macOS Metal Rendering Pipeline [v11]

Ambarish Rapte arapte at openjdk.org
Fri Aug 1 03:08:06 UTC 2025


On Thu, 31 Jul 2025 22:31:55 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> This issue is still unresolved: `CoreSymbols.getFunctions()` is a collection of `Function`, but it's searched for a `String` (.getName()).
>> 
>> The code in lines 216 and 238 needs to be fixed.
>> 
>> I suspect we are missing a unit test that exercises this functionality, because the code should never work.  How did it work?
>
> Andy and Nir are right. I don't see how the following can do what you intend:
> 
> 
> if (!CoreSymbols.getFunctions().contains(getFuncName(e.getFunction().getName())) &&
> 
> 
> Since `CoreSymbols.getFunctions()` is a `Set<Function>` and `e.getFunction().getName()` is a `String`, the `contains` will always return false (meaning this term in the `if` expression is always true).
> 
> Can you check this?

The check was incorrect and the `contains()` call always returned `false`. 
Due to this incorrect `contains()` call, I would have created the `libraryFunctionsUsedInShader` Set. `libraryFunctionsUsedInShader` is a subset of `CoreSymbols.getFunctions()`. So, after correcting the above `contains` call, the `libraryFunctionsUsedInShader` set is not required.

The previous if condition

if (!CoreSymbols.getFunctions().contains(getFuncName(d.getFunction().getName())) &&
    !libraryFunctionsUsedInShader.contains(getFuncName(d.getFunction().getName())))


is now changed to:

if (!CoreSymbols.getFunctions().contains(d.getFunction()))


The wrong check did not cause any issue, as the check `!CoreSymbols.getFunctions().contains(getFuncName(d.getFunction().getName()))` was wrong but always `true` and `!libraryFunctionsUsedInShader.contains(getFuncName(d.getFunction().getName()))'  check was sufficient enough for achieving required result.

The metal shaders generated before and after this change are exactly same.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1824#discussion_r2246784569


More information about the openjfx-dev mailing list