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

x/exp/shiny: assigning to *buf.RGBA() works on gldriver but not x11driver #14191

Closed
jnjackins opened this issue Feb 2, 2016 · 4 comments
Closed

Comments

@jnjackins
Copy link
Contributor

I had some code like this:

*buf.RGBA() = *img
tx.Upload(image.ZP, buf, buf.Bounds())
win.Copy(min, tx, tx.Bounds(), screen.Src, nil)

This worked on gldriver, but not x11driver. After switching to the following, it worked on both:

draw.Draw(buf.RGBA(), img.Bounds(), img, image.ZP, draw.Src)
tx.Upload(image.ZP, buf, buf.Bounds())
win.Copy(min, tx, tx.Bounds(), screen.Src, nil)

I have not yet investigated why it works on one but not the other, but first: should this work? It feels like I'm paying a performance penalty by drawing my image to buf.RGBA() when I already have exactly the image that I want.

@jnjackins
Copy link
Contributor Author

CC @nigeltao @crawshaw

@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Feb 2, 2016
@nigeltao
Copy link
Contributor

nigeltao commented Feb 4, 2016

The short answer is, no, it should not work.

A Buffer is essentially an *image.RGBA, but not all *image.RGBAs are valid Buffers. (If they were, the Upload method might as well take an *image.RGBA instead of a Buffer). Drivers may assume that the memory backing a Buffer's pixels are specially allocated, such as shared memory, shared across multiple processes. Arbitrary *image.RGBAs do not satisfy that assumption.

As I tried to say on #14026, your Editor shouldn't have an *image.RGBA or be an *image.RGBA. Instead, it should be able to render itself onto a Buffer's *image.RGBA (where the Buffer is provided by Screen.NewBuffer).

On a side note, with your original code snippet:

*buf.RGBA() = *img
tx.Upload(image.ZP, buf, buf.Bounds())
win.Copy(min, tx, tx.Bounds(), screen.Src, nil)

you might not need to explicitly use a Texture. A Window already has an Upload method, which takes a Buffer.

@jnjackins
Copy link
Contributor Author

As I tried to say on #14026, your Editor shouldn't have an *image.RGBA or be an *image.RGBA. Instead, it should be able to render itself onto a Buffer's *image.RGBA (where the Buffer is provided by Screen.NewBuffer).

Ah, thank you. This is obvious now that you say it (I missed it last time, apparently). I've been stuck on the idea that I need to construct my own image, and then copy it to the buffer.

you might not need to explicitly use a Texture. A Window already has an Upload method, which takes a Buffer.

Yes, my example code was simplified. Usually I can reuse the texture (which I believe is hypothetically cheaper than uploading the buffer each time).

@gopherbot
Copy link

CL https://golang.org/cl/19211 mentions this issue.

gopherbot pushed a commit to golang/exp that referenced this issue Feb 5, 2016
A Buffer is essentially an *image.RGBA, but not all *image.RGBAs are
valid Buffers. This change detects invalid attempts to pass arbitrary
*image.RGBAs as Buffers. Prior to this change, this would 'work' for the
gldriver but not other drivers.

Fixes golang/go#14191.

Change-Id: I8b0d7f0861dc69b93d332bd0ba26a70326e16c7b
Reviewed-on: https://go-review.googlesource.com/19211
Reviewed-by: David Crawshaw <crawshaw@golang.org>
@golang golang locked and limited conversation to collaborators Feb 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants