Skip to content

Comments

update the spec for blitting pixels outside the read buffer#2070

Merged
kenrussell merged 1 commit intoKhronosGroup:masterfrom
Richard-Yunchao:spec-blitframebuffer-outofbounds
Sep 29, 2016
Merged

update the spec for blitting pixels outside the read buffer#2070
kenrussell merged 1 commit intoKhronosGroup:masterfrom
Richard-Yunchao:spec-blitframebuffer-outofbounds

Conversation

@Richard-Yunchao
Copy link

@zhenyao , @kenrussell , PTAL. Thanks a lot!

@kenrussell
Copy link
Member

Thank you Yunchao for writing up this clarification.

@kenrussell kenrussell merged commit 791e8c6 into KhronosGroup:master Sep 29, 2016
the source are converted to a single sample before being written to the destination.

For any source pixel lying outside the read framebuffer, the corresponding destination pixel remains
untouched.
Copy link
Contributor

@zhenyao zhenyao Sep 29, 2016

Choose a reason for hiding this comment

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

What about pixels in the destination that their corresponding src pixels (in LINEAR mode) is partially inside and partially outside? The spec should cover all these cases without ambiguity.

Copy link
Member

Choose a reason for hiding this comment

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

It's a good question. Looking back at the old Khronos bug https://cvs.khronos.org/bugzilla/show_bug.cgi?id=3576 (sorry, non-public) linked from http://crbug.com/644740 , defining the math for the coordinate system transformations seemed difficult, and the OpenGL ES working group themselves opted for simpler spec language.

@Kangz do you have any suggestion about how to phrase this?

There does seem to be some ambiguity about what happens with pixels on the border of the source rectangle. For a LINEAR blit, CLAMP_TO_EDGE filtering should presumably be used, so that even if the source rectangle is fully inside the source framebuffer, pixels on the other side aren't referenced at all.

Copy link
Author

Choose a reason for hiding this comment

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

For pixels lying on the border, if the majority of the pixels is in the blit region, I think it should be blitted for NEAREST filter. And it should be always blitted for pixels lying on the border for LINEAR filter. WDYT? @zhenyao , @kenrussell , @Kangz ?

This Chromium revision will follow this criteria: https://codereview.chromium.org/2409523002/.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think for NEAREST or LINEAR, the destination pixels should be fully within the adjusted region (due to src being not fully within the read buffer bound).

Copy link
Contributor

Choose a reason for hiding this comment

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

@zhenyao the spec includes special language to say that GL_LINEAR filtering might cause reads outside of the src region in that case.
What we want is for a dest pixel to be written iff it is in the intersection of the the draw framebuffer bounds and the dest region, and its corresponding sample point is in the intersection of the read framebuffer and the source region ([x0, x1] x [y0, y1]). sampling at x0 + y0 will access source pixels at position (x0 - 0.5, y0 - 0.5) or clamp to edge.

My take on the clarification:
A destination pixel is written only if its corresponding sample point is included both in the source region and the source framebuffer bounds.

Copy link
Contributor

Choose a reason for hiding this comment

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

Deleted my comment. I agree with @Kangz on the clarification. Except I think sampling for target pixel (x0, y0) will be sampling at src point (x0 + 0.5, y0 + 0.5) if 1:1 scaling between src:dst is assumed.

Copy link
Author

Choose a reason for hiding this comment

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

I agree with @zhenyao. Let's discuss this issue at #2098.

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.

4 participants