KhronosGroup / KhronosGroup/OpenCL-CTS

Clean up slice padding logic in clCopyImage

Open
#1,429 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
232
Forks
235
Avg merge
8d 7h
Merged PRs (30d)
18

Description

From #1258 review comment https://github.com/KhronosGroup/OpenCL-CTS/pull/1258/files#r696781507, referring to the `mappedSlicedPad` computation in `create_image`:

> this whole construction is very awkward since mappedSlicePad is also wrong for 1D images (since mappedSlice must be 0 for 1D). It is used two different ways: to determine if the whole image can be copied in one go, and to implement the line-by-line copy for 2Darray and 3D images only. This requires extra logic in the if condition of line 247 and the (incomplete) comment in line 275 to mention that it's wrong for 2D images:
> // mappedSlicePad is incorrect for 2D images here, but we will exit the z loop before this is a problem.
> The comment doesn't mention that it's also wrong for 1D and 1Darray images as well.
>
> Although this fix works, I'd propose cleaning up a bit to implement a boolean called something like isMappedSlicePadded, which is set to true only for slice-padded 2Darray or 3D images. Then we can get rid of the (mappedSlicePad==0 || (imageInfo->depth==0 && imageInfo->arraySize==0)) expression below and replace it with !isMappedSlicedPadded. Since the numeric value of mappedSlicePad is only used in the line-by-line copy loop, we could simply do the calculation in the loop and note that it's computing slice padding for 2Darray and 3D images.

Should check other tests for similar logic and provide a common fix.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start at the create_image logic used by clCopyImage and trace mappedSlicedPad through the whole-image and line-by-line copy paths. Check other tests for similar slice-padding logic, then verify that the cleanup preserves copying for 1D, 1D-array, 2D, 2D-array, and 3D images.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
testing-qa
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.