Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2527343002:
Avoid doing and then abandoning semi-accelerated texImageHelperHTMLVideoElement path. (Closed)
|
Created:
4 years ago by aleksandar.stojiljkovic Modified:
4 years ago CC:
chromium-reviews, blink-reviews, haraken Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAvoid doing and then abandoning semi-accelerated texImageHelperHTMLVideoElement path.
This resolves performance issue when uploading video to e.g. FLOAT or
UNSIGNED_SHORT textures. If canUseCopyTextureCHROMIUM returns false, we
shouldn't create AcceleratedImageBufferSurface, call paintCurrentFrame and
texImage2DBase since they would not be used. This is because imageBuffer->
copyToPlatformTexture is not supported when canUseCopyTextureCHROMIUM is false
and it would early return false.
BUG=624436
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Committed: https://crrev.com/59c7989ec2c365f0bbaf09ecc02da7225061725b
Cr-Commit-Position: refs/heads/master@{#434926}
Patch Set 1 #
Total comments: 2
Messages
Total messages: 20 (15 generated)
Description was changed from ========== Avoid doing and then abandoning semi-accelerated texImageHelperHTMLVideoElement path. This resolves performance issue when uploading video to e.g. FLOAT or UNSIGNED_SHORT textures. If canUseCopyTextureCHROMIUM returns false, we shouldn't create AcceleratedImageBufferSurface, call paintCurrentFrame and texImage2DBase since they would not be used because imageBuffer-> copyToPlatformTexture would early return false (canUseCopyTextureCHROMIUM check). BUG=624436 ========== to ========== Avoid doing and then abandoning semi-accelerated texImageHelperHTMLVideoElement path. This resolves performance issue when uploading video to e.g. FLOAT or UNSIGNED_SHORT textures. If canUseCopyTextureCHROMIUM returns false, we shouldn't create AcceleratedImageBufferSurface, call paintCurrentFrame and texImage2DBase since they would not be used because imageBuffer-> copyToPlatformTexture would early return false (canUseCopyTextureCHROMIUM check). BUG=624436 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
The CQ bit was checked by aleksandar.stojiljkovic@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Avoid doing and then abandoning semi-accelerated texImageHelperHTMLVideoElement path. This resolves performance issue when uploading video to e.g. FLOAT or UNSIGNED_SHORT textures. If canUseCopyTextureCHROMIUM returns false, we shouldn't create AcceleratedImageBufferSurface, call paintCurrentFrame and texImage2DBase since they would not be used because imageBuffer-> copyToPlatformTexture would early return false (canUseCopyTextureCHROMIUM check). BUG=624436 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Avoid doing and then abandoning semi-accelerated texImageHelperHTMLVideoElement path. This resolves performance issue when uploading video to e.g. FLOAT or UNSIGNED_SHORT textures. If canUseCopyTextureCHROMIUM returns false, we shouldn't create AcceleratedImageBufferSurface, call paintCurrentFrame and texImage2DBase since they would not be used. This is because imageBuffer-> copyToPlatformTexture is not supported when canUseCopyTextureCHROMIUM is false and it would early return false. BUG=624436 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)
The CQ bit was checked by aleksandar.stojiljkovic@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
aleksandar.stojiljkovic@intel.com changed reviewers: + bajones@chromium.org, kbr@chromium.org, zmo@chromium.org
This patch is orthogonal to other changes in https://crrev.org/2476693002/ (16-bit depth video to float texture) so extracting it here as a separate patch. Thanks. https://codereview.chromium.org/2527343002/diff/1/third_party/WebKit/Source/m... File third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp (right): https://codereview.chromium.org/2527343002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:5190: if (imageBuffer->copyToPlatformTexture( Copy from the CL description: > shouldn't create AcceleratedImageBufferSurface, call paintCurrentFrame and texImage2DBase since they would not be used because imageBuffer->copyToPlatformTexture would early return when canUseCopyTextureCHROMIUM is false.
Thanks for clarifying this and splitting it into its own CL. LGTM https://codereview.chromium.org/2527343002/diff/1/third_party/WebKit/Source/m... File third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp (right): https://codereview.chromium.org/2527343002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:5190: if (imageBuffer->copyToPlatformTexture( On 2016/11/25 12:00:04, aleksandar.stojiljkovic wrote: > Copy from the CL description: > > shouldn't create AcceleratedImageBufferSurface, call paintCurrentFrame and > texImage2DBase since they would not be used because > imageBuffer->copyToPlatformTexture would early return when > canUseCopyTextureCHROMIUM is false. I see. Thanks for pointing that out.
The CQ bit was checked by aleksandar.stojiljkovic@intel.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1480400327348440, "parent_rev":
"21cb305f4631f0f9ec3017b40f27b000cd02c387", "commit_rev":
"861dff2ed55152de12cb5a2fa35727392c57211c"}
Message was sent while issue was closed.
Description was changed from ========== Avoid doing and then abandoning semi-accelerated texImageHelperHTMLVideoElement path. This resolves performance issue when uploading video to e.g. FLOAT or UNSIGNED_SHORT textures. If canUseCopyTextureCHROMIUM returns false, we shouldn't create AcceleratedImageBufferSurface, call paintCurrentFrame and texImage2DBase since they would not be used. This is because imageBuffer-> copyToPlatformTexture is not supported when canUseCopyTextureCHROMIUM is false and it would early return false. BUG=624436 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Avoid doing and then abandoning semi-accelerated texImageHelperHTMLVideoElement path. This resolves performance issue when uploading video to e.g. FLOAT or UNSIGNED_SHORT textures. If canUseCopyTextureCHROMIUM returns false, we shouldn't create AcceleratedImageBufferSurface, call paintCurrentFrame and texImage2DBase since they would not be used. This is because imageBuffer-> copyToPlatformTexture is not supported when canUseCopyTextureCHROMIUM is false and it would early return false. BUG=624436 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Avoid doing and then abandoning semi-accelerated texImageHelperHTMLVideoElement path. This resolves performance issue when uploading video to e.g. FLOAT or UNSIGNED_SHORT textures. If canUseCopyTextureCHROMIUM returns false, we shouldn't create AcceleratedImageBufferSurface, call paintCurrentFrame and texImage2DBase since they would not be used. This is because imageBuffer-> copyToPlatformTexture is not supported when canUseCopyTextureCHROMIUM is false and it would early return false. BUG=624436 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Avoid doing and then abandoning semi-accelerated texImageHelperHTMLVideoElement path. This resolves performance issue when uploading video to e.g. FLOAT or UNSIGNED_SHORT textures. If canUseCopyTextureCHROMIUM returns false, we shouldn't create AcceleratedImageBufferSurface, call paintCurrentFrame and texImage2DBase since they would not be used. This is because imageBuffer-> copyToPlatformTexture is not supported when canUseCopyTextureCHROMIUM is false and it would early return false. BUG=624436 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Committed: https://crrev.com/59c7989ec2c365f0bbaf09ecc02da7225061725b Cr-Commit-Position: refs/heads/master@{#434926} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/59c7989ec2c365f0bbaf09ecc02da7225061725b Cr-Commit-Position: refs/heads/master@{#434926} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
