Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy depth aspect by rendering in overdraw #2079

Merged
merged 3 commits into from
Jul 27, 2018

Conversation

sean-purcell
Copy link
Contributor

Specifically do this for the store end (i.e. new depth/stencil image to
old image), so that we don't have to add TRANSFER_DST to all depth
images.

Additionally, remove the conditions in gapii and always add the transfer_src flag to images, so that we can use it to copy from at the start of the overdraw renderpass.

@@ -1047,44 +1074,45 @@ func (h *ipRenderHandler) free() {
}
}

func imageBarrierAspectFlags(aspect VkImageAspectFlagBits, fmt VkFormat) VkImageAspectFlags {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I'd prefer name it ipImageBarrierAspectFlags. imageBarrierAspectFlags sounds quite general to me, and should be reserved for api utility subroutines or extern functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

memory.Nullptr,
0,
memory.Nullptr,
1,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uint32(len(dstBarriers))?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

input.image.Info().ArrayLayers(), // layerCount
),
))
dstBarriers = append(dstBarriers,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Prefer to have a check to see if the finalLayout and the current layout is the same. Because for primeByRendering jobs, the layout of input attachments are guaranteed to be SHADER_READ_ONLY_OPTIMAL before the rendering (line 80), so this is not necessary for those cases, also the barriers for the input attachments in the inputSrcBarriers above.

  2. When render() is called from places other than primeByRendering for aspects other than ONLY stencil, not used when priming for COLOR or DEPTH aspects, so that layout transitions for input attachments can be missed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, the 2) point is not valid, and for the 1) not necessary for the inputSrcBarriers, I misunderstood the code here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Specifically do this for the store end (i.e. new depth/stencil image to
old image), so that we don't have to add TRANSFER_DST to all depth
images.
@Qining Qining requested a review from AWoloszyn July 24, 2018 15:52
@Qining
Copy link
Contributor

Qining commented Jul 24, 2018

image_primer code looks good to me. Defer the other parts to @AWoloszyn .

@sean-purcell
Copy link
Contributor Author

Also adding a fix for an incorrect handling of clear values that led to the stencil aspect being cleared to the wrong value sometimes.

@AWoloszyn AWoloszyn merged commit 6583f9b into google:master Jul 27, 2018
@sean-purcell sean-purcell deleted the overdraw-render branch July 27, 2018 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants