1488 2024-01-22 02:36:55 +0100 Graph: Resolve Performance Regression in RegionRenderer's ShaderKey Utilization 2024-01-25 09:22:19 +0100 1 1 3 JogAmp Jogl graph 2.6.0 All all RESOLVED FIXED P1 critical --- 805 1 sgothel sgothel sgothel DEFECT ffe4e670c9d35121934c6f3c95067d9c18aee386 6cf158cc59e901b49ab54681e363d23492421a9d 1dcfdf71c09c6d774ac47012c05e09da4a085d7b 95e2519a0fc37d19d05bc0f4cc14f593eaae7dd9 b711ae5239b8581a197d468b2804cfeb8c4d6c94 --- oldest_to_newest 7049 0 sgothel 2024-01-22 02:36:55 +0100 RegionRenderer's ShaderKey Commit 6363ae5fb6975a6f2e7c1093ce81f25b699e3e61 changed RegionRenderer.useShaderProgram()'s shader mapping using a new ShaderKey instance. Such ShaderKey instance is created every time @ RegionRenderer.useShaderProgram() to retrieve the ShaderProgram from the HashMap<ShaderKey, Shader Program>. While this is most correct, creating the ShaderKey instance causes a lot of temp objects. ShaderKey also uses the optional colorTexSeq shader code for equality test in case of hash-collisions. Previous code simply ignored hash-collisions and used a 1:1 hashCode -> ShaderProgram mapping using our IntObjectHashMap. However, there was no test whether collision occur. +++ Solution would be either 1- Revert fully to the previous code just using an IntObjectHashMap, but throwing a RuntimeException in case of hashCode collisions. In case of a collisions, we would need to produce a better hashCode. This is possible, as the underlying data is fully internal .. etc. 2- Use the IntObjectHashMap as long there is no hashCode collision, then revert back to HashMap<ShaderKey, Shader Program>. I favor variant (1), if it is possible to validate the hashCode. 7054 1 sgothel 2024-01-22 05:57:05 +0100 commit ffe4e670c9d35121934c6f3c95067d9c18aee386 implements solution 1, i.e. throws exception on shader hash-code collision. Code supplies both variants, however, after already having used IntObjectHashMap w/o actual ShaderKey artifacts we have to think about whether fallback is even possible. Otherwise .. in case we have no internal collision, this should be good enough. 7058 2 sgothel 2024-01-22 06:45:04 +0100 commit 6cf158cc59e901b49ab54681e363d23492421a9d (HEAD -> master) Bug 1488 - Graph RegionRenderer: Ensure shaderPrograms1 path is disabled using 'static final boolean useShaderPrograms0 = true' 7060 3 sgothel 2024-01-22 08:39:05 +0100 Closing for now - may need to be reopened in case commit ffe4e670c9d35121934c6f3c95067d9c18aee386 is not sufficient, see comment 1 7061 4 sgothel 2024-01-22 22:03:21 +0100 commit 1dcfdf71c09c6d774ac47012c05e09da4a085d7b RegionRenderer: Use a more deterministic 64-bit shaderKey: [ 0-31] bit values and state, [32-63] colorTexSeqHash This leaves only room for a key collision on the 32-bit colorTexSeqHash value and hence should be save within our shader-code environment. + // # | s | + // 0 | 1 | isTwoPass + // 1 | 1 | pass1 + // 2 | 5 | ShaderModeSelector1 + // 7 | 1 | hasFrustumClipping + // 8 | 1 | hasColorChannel + // 9 | 1 | hasColorTexture + // 32 | 32 | colorTexSeqHash + long hash = ( isTwoPass ? 1 : 0 ); + hash = ( hash << 1 ) | ( pass1 ? 1 : 0 ) ; + hash = ( hash << 1 ) | sms.ordinal(); // incl. pass2Quality + sampleCount + hash = ( hash << 5 ) | ( hasFrustumClipping ? 1 : 0 ); + hash = ( hash << 1 ) | ( hasColorChannel ? 1 : 0 ); + hash = ( hash << 1 ) | ( hasColorTexture ? 1 : 0 ); + hash = ( hash << 1 ) | ( ( colorTexSeqHash & 0xFFFFFFL ) << 32 ); 7062 5 sgothel 2024-01-22 22:05:22 +0100 commit 95e2519a0fc37d19d05bc0f4cc14f593eaae7dd9 FFMPEGMediaPlayer: Fix getTextureFragmentShaderHashID(), i.e. use actual tc_w_1 = (float)getWidth() / (float)texWidth value as hardcoded within the shader 7065 6 sgothel 2024-01-25 09:22:19 +0100 (In reply to Sven Gothel from comment #4) commit b711ae5239b8581a197d468b2804cfeb8c4d6c94 Bug 1488: Complete/Fix producing the 64-bit shaderKey: Use long values in bit-shift expressions and simplify it commit 1dcfdf71c09c6d774ac47012c05e09da4a085d7b - still used the 'hash code' bit shift pattern, not necessary -> simplified - the value as not ensured to be long, hence conversion occured This caused Graph's MSAA not being picked up properly using the shaderKey. - long hash = ( isTwoPass ? 1 : 0 ); - hash = ( hash << 1 ) | ( pass1 ? 1 : 0 ) ; - hash = ( hash << 1 ) | sms.ordinal(); // incl. pass2Quality + sampleCount - hash = ( hash << 5 ) | ( hasFrustumClipping ? 1 : 0 ); - hash = ( hash << 1 ) | ( hasColorChannel ? 1 : 0 ); - hash = ( hash << 1 ) | ( hasColorTexture ? 1 : 0 ); - hash = ( hash << 1 ) | ( ( colorTexSeqHash & 0xFFFFFFL ) << 32 ); + long hash = isTwoPass ? 1L : 0L; + hash |= ( pass1 ? 1L : 0L ) << 1; + hash |= (long)( sms.ordinal() & 0b11111 ) << 2; // incl. pass2Quality + sampleCount + hash |= ( hasFrustumClipping ? 1L : 0L ) << 7; + hash |= ( hasColorChannel ? 1L : 0L ) << 8; + hash |= ( hasColorTexture ? 1L : 0L ) << 9; + hash |= ( colorTexSeqHash & 0xFFFFFFL ) << 32;