Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v4]
On Sun, 4 Apr 2021 10:00:16 GMT, Jayathirth D V wrote: >>> We need to use appropriate encoder(where scissor is set) to honour clip in >>> copyArea. >> >> It is quite interesting that blitting with or without clipping does not have >> any difference. thank you for confirmation. but probably our perf tests are >> not good enough. > > @mrserb Are there any more inputs/review comments? nop, it is fine. - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v4]
On Sat, 3 Apr 2021 05:30:12 GMT, Sergey Bylokhov wrote: >>> > I am not getting what do you mean by 'new "round trip" '. Please >>> > elaborate. >>> > Regarding performance metrics i have captured the details in the bug and >>> > there is no degradation. >>> >>> The new code path which takes care of the clip, if there is no degradation >>> means that we get it for free? >> >> Before this change we used to get blitEncoder from same commandbuffer(and >> same CommandQueue) as we are doing now. And then commit the commandbuffer, >> so from computational perspective we are not holding or doing anything extra >> in new implementation. We need to use appropriate encoder(where scissor is >> set) to honour clip in copyArea. >> >> Since we are not seeing any difference in performance numbers (especially in >> swingmark where we do lot of scrolling/copyArea) we are basically getting >> clipping in copyArea without any degradation. Also in Netbeans i have done >> good amount scrolling test and i dont see any degradation. Also i expected >> improvement in performance (we are seeing little improvement in swingmark >> numbers) because we are actually doing less amount of copy operation now. > >> We need to use appropriate encoder(where scissor is set) to honour clip in >> copyArea. > > It is quite interesting that blitting with or without clipping does not have > any difference. thank you for confirmation. but probably our perf tests are > not good enough. @mrserb Are there any more inputs/review comments? - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v4]
On Sat, 3 Apr 2021 05:30:12 GMT, Sergey Bylokhov wrote: > > We need to use appropriate encoder(where scissor is set) to honour clip in > > copyArea. > > It is quite interesting that blitting with or without clipping does not have > any difference. thank you for confirmation. but probably our perf tests are > not good enough. Exactly i suspect we are not hitting threshold of GPU computing in our tests. If we hit GPU threshold with larger clip bounds i expect performance to be on higher side after the fix. - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v4]
On Sat, 3 Apr 2021 05:02:24 GMT, Jayathirth D V wrote: > We need to use appropriate encoder(where scissor is set) to honour clip in > copyArea. It is quite interesting that blitting with or without clipping does not have any difference. thank you for confirmation. but probably our perf tests are not good enough. - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v4]
On Sat, 3 Apr 2021 03:51:29 GMT, Sergey Bylokhov wrote: > > I am not getting what do you mean by 'new "round trip" '. Please elaborate. > > Regarding performance metrics i have captured the details in the bug and > > there is no degradation. > > The new code path which takes care of the clip, if there is no degradation > means that we get it for free? Before this change we used to get blitEncoder from same commandbuffer(and same CommandQueue) as we are doing now. And then commit the commandbuffer, so from computational perspective we are not holding or doing anything extra in new implementation. We need to use appropriate encoder(where scissor is set) to honour clip in copyArea. Since we are not seeing any difference in performance numbers (especially in swingmark where we do lot of scrolling/copyArea) we are basically getting clipping in copyArea without any degradation. Also in Netbeans i have done good amount scrolling test and i dont see any degradation. - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v4]
On Sat, 3 Apr 2021 02:56:48 GMT, Jayathirth D V wrote: > I am not getting what do you mean by 'new "round trip" '. Please elaborate. > Regarding performance metrics i have captured the details in the bug and > there is no degradation. The new code path which takes care of the clip, if there is no degradation means that we get it for free? - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v4]
On Fri, 2 Apr 2021 22:43:36 GMT, Sergey Bylokhov wrote: > Just curious how this new "round trip" will affect the performance when the > clip is set and when not? is it cheap? I am not getting what do you mean by 'new "round trip" '. Please elaborate. Regarding performance metrics i have captured the details in the bug and there is no degradation. - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v4]
On Thu, 1 Apr 2021 15:50:50 GMT, Prasanta Sadhukhan wrote: >> Jayathirth D V has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Refactor code to use drawTex2Tex > > Marked as reviewed by psadhukhan (Reviewer). Just curious how this new "round trip" will affect the performance when the clip is set and when not? is it cheap? - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v4]
On Thu, 1 Apr 2021 14:15:37 GMT, Jayathirth D V wrote: >> In MTLBlitLoops.copyArea() we use standalone encoder which has no clip state >> information because of which we ignore clip parameters set in rect clip and >> shape clip. We need to query and use encoders from EncoderManager to honour >> clip states in copyArea. > > Jayathirth D V has updated the pull request incrementally with one additional > commit since the last revision: > > Refactor code to use drawTex2Tex Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v4]
> In MTLBlitLoops.copyArea() we use standalone encoder which has no clip state > information because of which we ignore clip parameters set in rect clip and > shape clip. We need to query and use encoders from EncoderManager to honour > clip states in copyArea. Jayathirth D V has updated the pull request incrementally with one additional commit since the last revision: Refactor code to use drawTex2Tex - Changes: - all: https://git.openjdk.java.net/jdk/pull/3283/files - new: https://git.openjdk.java.net/jdk/pull/3283/files/9b4a90ab..91e068b9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3283&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3283&range=02-03 Stats: 35 lines in 1 file changed: 0 ins; 22 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/3283.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3283/head:pull/3283 PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v3]
On Thu, 1 Apr 2021 06:58:27 GMT, Prasanta Sadhukhan wrote: >> Jayathirth D V has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Comment update > > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLBlitLoops.m line > 826: > >> 824: id interEncoder = >> 825: [mtlc.encoderManager getTextureEncoder:interTexture >> 826:isSrcOpaque:dstOps->isOpaque > > Should it not be srcOps->isOpaque? Both source and destination are same in copyArea. So isOpaque property will be same. But we use 32 bit non-opaque intermediate texture, so for intermediate texture we should pass JNI_FALSE for isOpaque property. > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLBlitLoops.m line > 854: > >> 852: atIndex:MeshVertexBuffer]; >> 853: [finalEncoder setFragmentTexture:interTexture atIndex: 0]; >> 854: [finalEncoder drawPrimitives:MTLPrimitiveTypeTriangle >> vertexStart:0 > > Can't we reuse drawTex2Tex() for this snippet? Thanks for the suggestion Prasanta, yes we should refactor code to use drawTex2Tex. I have updated the PR. - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v3]
On Thu, 1 Apr 2021 05:49:51 GMT, Jayathirth D V wrote: >> In MTLBlitLoops.copyArea() we use standalone encoder which has no clip state >> information because of which we ignore clip parameters set in rect clip and >> shape clip. We need to query and use encoders from EncoderManager to honour >> clip states in copyArea. > > Jayathirth D V has updated the pull request incrementally with one additional > commit since the last revision: > > Comment update src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLBlitLoops.m line 826: > 824: id interEncoder = > 825: [mtlc.encoderManager getTextureEncoder:interTexture > 826:isSrcOpaque:dstOps->isOpaque Should it not be srcOps->isOpaque? src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLBlitLoops.m line 854: > 852: atIndex:MeshVertexBuffer]; > 853: [finalEncoder setFragmentTexture:interTexture atIndex: 0]; > 854: [finalEncoder drawPrimitives:MTLPrimitiveTypeTriangle > vertexStart:0 Can't we reuse drawTex2Tex() for this snippet? - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v3]
On Thu, 1 Apr 2021 05:49:51 GMT, Jayathirth D V wrote: >> In MTLBlitLoops.copyArea() we use standalone encoder which has no clip state >> information because of which we ignore clip parameters set in rect clip and >> shape clip. We need to query and use encoders from EncoderManager to honour >> clip states in copyArea. > > Jayathirth D V has updated the pull request incrementally with one additional > commit since the last revision: > > Comment update Marked as reviewed by aghaisas (Committer). - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v3]
> In MTLBlitLoops.copyArea() we use standalone encoder which has no clip state > information because of which we ignore clip parameters set in rect clip and > shape clip. We need to query and use encoders from EncoderManager to honour > clip states in copyArea. Jayathirth D V has updated the pull request incrementally with one additional commit since the last revision: Comment update - Changes: - all: https://git.openjdk.java.net/jdk/pull/3283/files - new: https://git.openjdk.java.net/jdk/pull/3283/files/3e1ba4c9..9b4a90ab Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3283&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3283&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3283.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3283/head:pull/3283 PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v2]
On Wed, 31 Mar 2021 15:03:54 GMT, Jayathirth D V wrote: >> In MTLBlitLoops.copyArea() we use standalone encoder which has no clip state >> information because of which we ignore clip parameters set in rect clip and >> shape clip. We need to query and use encoders from EncoderManager to honour >> clip states in copyArea. > > Jayathirth D V has updated the pull request incrementally with one additional > commit since the last revision: > > Add comment on usage of MTLRenderCommandEncoder src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLBlitLoops.m line 821: > 819: * performing copyArea, thats why we need to query encoder > with > 820: * appropriate state from EncoderManager and not use > 821: * direct MTLBlitCommandEncoder for texture mapping. Minor : "texture mapping" should be "texture copy" as MTLBlitCommandEncoder cannot be used for texture mapping anyway. - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline
On Wed, 31 Mar 2021 12:39:49 GMT, Ajit Ghaisas wrote: > I tested this fix. No regressions were observed. > > Using MTLBlitCommandEncoder is intuitive when metal resource copying needs to > be done. > I suggest to add a comment (may be as a method description or in code where > we do the actual copy) to mention why we use MTLRenderCommandEncoder and not > MTLBlitCommandEncoder. Thanks Ajit for testing the fix. I have added comment mentioning why we use MTLRenderCommandEncoder and not MTLBlitCommandEncoder. - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v2]
> In MTLBlitLoops.copyArea() we use standalone encoder which has no clip state > information because of which we ignore clip parameters set in rect clip and > shape clip. We need to query and use encoders from EncoderManager to honour > clip states in copyArea. Jayathirth D V has updated the pull request incrementally with one additional commit since the last revision: Add comment on usage of MTLRenderCommandEncoder - Changes: - all: https://git.openjdk.java.net/jdk/pull/3283/files - new: https://git.openjdk.java.net/jdk/pull/3283/files/d72f9490..3e1ba4c9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3283&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3283&range=00-01 Stats: 7 lines in 1 file changed: 7 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3283.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3283/head:pull/3283 PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline
On Wed, 31 Mar 2021 07:27:45 GMT, Jayathirth D V wrote: > In MTLBlitLoops.copyArea() we use standalone encoder which has no clip state > information because of which we ignore clip parameters set in rect clip and > shape clip. We need to query and use encoders from EncoderManager to honour > clip states in copyArea. I tested this fix. No regressions were observed. Using MTLBlitCommandEncoder is intuitive when metal resource copying needs to be done. I suggest to add a comment (may be as a method description or in code where we do the actual copy) to mention why we use MTLRenderCommandEncoder and not MTLBlitCommandEncoder. - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline
On Wed, 31 Mar 2021 07:27:45 GMT, Jayathirth D V wrote: > In MTLBlitLoops.copyArea() we use standalone encoder which has no clip state > information because of which we ignore clip parameters set in rect clip and > shape clip. We need to query and use encoders from EncoderManager to honour > clip states in copyArea. src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLTexurePool.m line 206: > 204:mipmapped:NO]; > 205: textureDescriptor.usage = MTLTextureUsageRenderTarget | > 206: MTLTextureUsageShaderRead; In copyArea we use intermediate texture queried from TexturePool as render target. If we dont set any MTLTextureUsage, we are getting assertion errors when Metal API validation is enabled because by default we can only do texture shader read(https://developer.apple.com/documentation/metal/mtltexturedescriptor/1515783-usage?language=objc). To avoid these assertion errors in Metal API validation we need to set appropriate MTLTextureUsage parameters. - PR: https://git.openjdk.java.net/jdk/pull/3283