Merge lp:~kdub/mir/overlay-gl-program into lp:mir

[Kevin DuBois]

android: add simple shader program that the android display system will use when the driver rejects the overlays. At the time being, it is just a simple blitter, as we start to support other HWC capabilites like alpha or 90 degree rotation, it will grow, but just a line or two.

android: add simple shader program that the android display system will use when the driver rejects the overlays. At the time being, it is just a simple blitter, as we start to support other HWC capabilites like alpha or 90 degree rotation, it will grow, but just a line or two.

note:
* The HWC is not being used by the platform-independent code. This code can potentially be used in the mir_demo_standalone_render_overlays program.

* I mention this somewhat a lot, but one more time won't hurt. We arrange a list of buffers and submit them to hwc's prepare() function. HWC can choose to accept or reject each buffer as a buffer that it can get to the screen in an optimized way. If the buffer is rejected, we're obliged to use OpenGL to render those buffers to a fb target that is also known to hwc. Although it might seem like we could use something other than OpenGL to draw (perhaps), hwc and OpenGL are pretty intrinsically tied together, doing so would cause driver problems.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1582
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1403/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1707
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1705
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1280/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1135
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1135/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1140/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1283/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1403/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1583
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1407/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1714
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1712
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1286
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1139
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1139/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1144
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1144/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1289
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1289/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1187
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6427

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1407/rebuild

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

(1) I think there's a little too much redundancy (and regression) between the existing GLRenderer and OverlayGLProgram. It would be better if we made use of the existing GLRenderer so we didn't have to fix the GL rendering logic twice every time we change it. Plus it makes sense that if overlay fails, then "use the GLRenderer you already know that works".

One thing this proposal regresses on is resize support:
295 +GLfloat const texcoords[]{
296 + 0.0f, 0.0f,
297 + 0.0f, 1.0f,
298 + 1.0f, 0.0f,
299 + 1.0f, 1.0f
300 +};
Fixed texcoords means you're regressing on bug 1259887.

(2) This is misleading:
  +void mga::OverlayGLProgram::render(
"program" has a very specific meaning in OpenGL but you are giving a "GLProgram" class fixed-function GL calls. That makes no sense. But also, see (1)

I suggest these changes would be better deferred until there's a real use case for them, because they only create serious maintenance concerns AFAICS right now. And with some more thought, we should fall back to the GLRenderer (or whatever the default renderer is), instead of writing yet another GL renderer.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

In fact, bypass does this already. If bypass fails at any stage it falls back to the GLRenderer. Overlays should do that too.

[Alan Griffiths (alan-griffiths)]

There's a lack of role clarity about our various GL classes but I don't mind a little duplication now if it gets factored out later.

OTOH Daniel makes a good point about GL "program".

review: Abstain

[Cemil Azizoglu (cemil-azizoglu)]

What is being rendered is "overlay rejects" and once rejected by the hwc, there is nothing "overlay" about them. These renderables should no longer be considered overlays, and the code to render them should not be called a name with the word "overlay" in it (unless it's overlay_rejects or smth that makes it clear that they are not overlays) for the sake of clarity. I see that there is code that already made it in, like render_gl_and_overlays, which I think is confusing and should be cleaned up in time.

For these rejects, I think it'd be nice to find a way to fall back to the usual path (I know you had some concerns about it, though). This may be a good time to factor things out of GLRenderer to make it reuseable. But I don't have an issue with a little bit of duplication if we want to wait for "role clarity", as Alan put it.

review: Needs Fixing

[Gerry Boland (gerboland)]

> What is being rendered is "overlay rejects" and once rejected by the hwc,
> there is nothing "overlay" about them. These renderables should no longer be
> considered overlays, and the code to render them should not be called a name
> with the word "overlay" in it (unless it's overlay_rejects or smth that makes
> it clear that they are not overlays) for the sake of clarity. I see that there
> is code that already made it in, like render_gl_and_overlays, which I think is
> confusing and should be cleaned up in time.

We chose this "list of surfaces to overlay" approach to avoid 2-way communication between the GL renderer (may not be Mir's GLRenderer remember) and the HWC code. This is especially important for case of nested server where a return message of "these surfaces were rejected by the hwc, so do it yourself" would be costly. From the shell point of view, the fire and forget approach is easier.

I also prefer having the simple OverlayGLProgram as the GL fallback, instead of the more resource heavy GLRenderer.

[Kevin DuBois (kdub)]

> What is being rendered is "overlay rejects" and once rejected by the hwc,
> there is nothing "overlay" about them. These renderables should no longer be
> considered overlays, and the code to render them should not be called a name
> with the word "overlay" in it (unless it's overlay_rejects or smth that makes
> it clear that they are not overlays) for the sake of clarity. I see that there
> is code that already made it in, like render_gl_and_overlays, which I think is
> confusing and should be cleaned up in time.

'and once rejected by the hwc, there is nothing "overlay" about them"
From the DefaultDisplayBufferCompositor's perspective, it was still a surface that was accepted as a hardware optimization, its a quirk of the HWC api that we actually do some of the rendering with GL.

There is a subtle, yet distinct difference between a mg::Renderable and an android overlay.

A list of mg::Renderable can do things that a list of hwc_display_contents_1 cannot.
The android code has a fixed function that it is expected to perform as a matter of HWC api.
RenderableList is the list that we can expand to do what we want our most advanced GL compositors to do. Some more concrete examples are that the HWC api expects an orthogonal projection, the RenderableList can use any perspective it wants. The RenderableList could potentially model things like a blur effect (or something) that the hwc would have no idea about. The RenderableList can set arbitrary transforms with arbitrarily tessellated surfaces, android can only support a 90degree rotation., etc.

The android code will take the more universal list and reject that list to the DisplayBufferCompositor if the RenderableList has a requirement that the HWC cannot optimize.

[Kevin DuBois (kdub)]

As for the "reuse or not to reuse" I considered this, and decided in favor of adding a very simple program like this.

1) Tying parts of the GLRenderer to the android-specific fallback renderer makes the GLRenderer parts not as malleable; they have to support the hard requirements that the Android fallback needs. I fear that a future bug-fixer or feature-implementer might not realize that changing GLRenderer stuff might break the hard requirements that this GL program has.

2) We might want to do something in the android shader that we doesn't have a concept in GLRenderer, like 'desaturate GPU-renderered regions for debugging purposes"

3) The duplication is really just the most primitive things a shader program can do; we either have 1 shader to rule them all or we have 'duplication' of lines like "gl_FragColor = texture2D(tex, v_texcoord);\n". I'd rather hedge on having different programs fulfilling the role we need them to fulfill as opposed to trying to have one program that we fit in different roles.

[Kevin DuBois (kdub)]

> (1) I think there's a little too much redundancy (and regression) between the
> existing GLRenderer and OverlayGLProgram. It would be better if we made use of
> the existing GLRenderer so we didn't have to fix the GL rendering logic twice
> every time we change it. Plus it makes sense that if overlay fails, then "use
> the GLRenderer you already know that works".

My concern is that I don't have to fix the Android program any time that renderable expands in a way that causes problems for the requirements that this code needs to support as a matter of the HWC api. I want to let the 'universal, platform-independant, flexible renderer" (the GLRenderer) be malleable while we try to solve those problems, while the android 'do-what-HWC-commands-me-to-do renderer" stays fixed, and abstracted from the code up in libmirserver.so

> One thing this proposal regresses on is resize support:
> 295 +GLfloat const texcoords[]{
> 296 + 0.0f, 0.0f,
> 297 + 0.0f, 1.0f,
> 298 + 1.0f, 0.0f,
> 299 + 1.0f, 1.0f
> 300 +};
> Fixed texcoords means you're regressing on bug 1259887.

Interesting, although its an exercise in the definition of 'regression' to figure out if new code with a new purpose can regress. It also points out another difference in the RenderableList and the hwc_contents_1_t list, which is that the hwc list defines position by the top-left corner, and the Renderable::transformation is a center-based positioning. This is a bit difficult to make sure that this doesn't have visual effects at the moment, because its not hooked up all the way through. I'll make sure to test this (hwc overlay+resize is giving me some other problems in my yet-proposed branches)

>
> (2) This is misleading:
> +void mga::OverlayGLProgram::render(
> "program" has a very specific meaning in OpenGL but you are giving a
> "GLProgram" class fixed-function GL calls. That makes no sense. But also, see
> (1)

ack, will think of new naming.

>
> I suggest these changes would be better deferred until there's a real use case
> for them, because they only create serious maintenance concerns AFAICS right
> now. And with some more thought, we should fall back to the GLRenderer (or
> whatever the default renderer is), instead of writing yet another GL renderer.

The real use-case right now is the mir_demo_standalone_render_overlays, which won't render anything in some cases, depending on driver behavior. This will be hooked up in later reviews, but I thought I'd propose this now, as it seemed like a pretty contained MP. The android platform is a bit strange in that it needs this GL functionality to render overlays sometimes. I want the quirk of the android platform to be abstracted as much as we possible from the GLRenderer/DisplayBufferCompositor code.

[Cemil Azizoglu (cemil-azizoglu)]

Ok I understand the GLRenderer can get arbitrarily elaborate and it may not be a good idea to use the same objects/shaders to render both. I'm all for KISS! Also, I think you're saying GLRenderer could be replaced but "overlay rendering" (now I'm using the term that I said I didnt like) cannot, and making them share code might cause problems for those that want to replace the GLRenderer. Understood.

You said you'd change the class name above. Could we find a different designation for these surfaces once they 've been rejected as overlays.

[Cemil Azizoglu (cemil-azizoglu)]

AFAIS the this fallback renderer is not replaceable. This means that we 'd be requiring GL capable GPU once this code is merged. To me, it's okay, Android has required it for a couple of years now. But since GLRenderer is replaceable, perhaps there was a discussion/expectation that things would work without GL. So I wanted to explicitly state this.

[Kevin DuBois (kdub)]

> AFAIS the this fallback renderer is not replaceable. This means that we 'd be
> requiring GL capable GPU once this code is merged. To me, it's okay, Android
> has required it for a couple of years now. But since GLRenderer is
> replaceable, perhaps there was a discussion/expectation that things would work
> without GL. So I wanted to explicitly state this.

Right, a GL capable gpu, but really just for the android platform. We also have further detanglement to do in mg::DisplayBuffer if we ever get a non-GL/EGL platform.

Still thinking on the naming, but I think its good to make the distinction of 'overlays' just in the android platform, and, within the android platform, to classify by 'overlay' or by 'fallback rendered' based on the result of prepare()

[Daniel van Vugt (vanvugt)]

Can we establish what's wrong with GLRenderer? In theory it's going to be much faster (e.g. has texture caching), less buggy (e.g. the texcoords) and more featured (e.g. has screen rotation) than what's presented here. Using GLRenderer is not only faster, less buggy and more featured, but also halves maintenance effort.

GLRenderer appears to win at every point. So if anyone thinks it's heavy-weight then please point out where and we'll fix it.

[Alberto Aguirre (albaguirre)]

OverlayCompositor and OverlayGLProgram,

maybe
HWCFallbackCompositor, HWCFallbackRenderer?

[Kevin DuBois (kdub)]

> Can we establish what's wrong with GLRenderer? In theory it's going to be much
> faster (e.g. has texture caching), less buggy (e.g. the texcoords) and more
> featured (e.g. has screen rotation) than what's presented here. Using
> GLRenderer is not only faster, less buggy and more featured, but also halves
> maintenance effort.
>
> GLRenderer appears to win at every point. So if anyone thinks it's heavy-
> weight then please point out where and we'll fix it.

Right, so nothing is wrong with GLRenderer, but we do have difference in what we require the two programs to do. We have marching orders to make GLRenderer+QtSG the next-gen full featured composition system (that is also the USC renderer); something that is much more than the requirements HWC gives us to implement the overlay functionality.

Writing a second program is a bit of a judgement call and the upsides are that the GLRenderer can grow and flex without having to fulfill the "be the full-featured next generation advanced OpenGL compositor" requirement while still having to fulfill the fixed requirements of HWC. GLRenderer can just grow to be the advanced compositor we want it to be. I'd agree with the half the maintenence effort if this was an open-ended program, but its not. This program has a very small fixed set of things it has to do, so the road to the perfect HWC program is short and unambiguous.

GLRenderer still has a fair amount of expansion that we'll build upon, and we'll isolate this fallback path from any changes to that class. (changes that would only hit the android platform as bugs). I'm worried that we'll go to implement something like an animation system (on the desktop) and inadvertently cause rendering problems on android, problems that would only occur in fallback scenarios.

I think a decent compromise is to:
1) maintain the second small program in the android platform, so we can let GLRenderer grow and isolate this platform-specific GL requirement from any changes there.
2) tease out mc::GLRenderer::tessellate() (and perhaps other universal functionality) from GLRenderer so there's more code re-use.

[Daniel van Vugt (vanvugt)]

I'm concerned that we're going to have to fix everything twice. And in fact fix things that aren't yet broken, like resizing and rotation. Delegating blame of such issues to just say it only affects overlays is one thing, but fixing regressions will occupy more time than not having to fix regressions.

Also consider this: Most shells will presumably be built on OpenGL at some level. So unless you always have a fullscreen app open part of the shell will be visible. And if part of the shell is visible then you already have to do some GL rendering on that frame. And if you already have to GLRender on that frame, why not use the GLRenderer that is already going to be used for other elements in that frame anyway?

I share your fears (and some excitement) for the growth of GLRenderer. But we are capable of breaking it up and modularization. Having thought about animation a lot in the past year, if I believed it should be strictly wired into the renderer then I would have proposed it already (the design is mostly done in my head). However I too want to be cautious about how much responsibility the core default GLRenderer has.

On the issue of tessellate(), that's an interesting one. Because tessellate() is a strictly OpenGL-specific operation. Not just in its implementation, but the concept only makes sense to OpenGL in my experience. And having to produce only triangles is unfortunately a requirement only for the ES variation of OpenGL.

[Alberto Aguirre (albaguirre)]

I think I'm convinced that it should be a separate program.

However it should be a very simple fallback - that is no resize support and no handling of screen rotation; I believe what is proposed here satisfies that.

If those features are needed, then it should fallback to the path we currently have (with GLRenderer).

So: hwc->prepare(..)
If all layers accepted - great - nothing else to do.
If some layers got rejected and need screen rotation/resizing - fallback to GLRenderer path - composite all layers with GLRenderer - essentially the path we use currently.
Otherwise fallback so simple hwc fallback GL compositor.

Needs fixing:
OverlayGLProgram - my suggestion is HWCFallbackGLRenderer to make it real clear what is responsiblity is.
OverlayCompositor - like Cemil says once rejected they are not "overlays" per se. So my suggestions is HWCFallbackCompositor

review: Needs Information

[Cemil Azizoglu (cemil-azizoglu)]

+1 on the names Alberto is proposing.

I think things will get interesting when YUV support is eventually added. Hopefully display overlay will support the format so hwc will not reject it. But if it does, it's almost certain that resize will be needed - video dimensions almost never match player's viewing area. Also, rotation should be supported IMHO, too.

Why not use the "replaceable renderer"? (I'm not calling it GLRenderer on purpose). It may not be capable of handling YUV due to, perhaps, hardware limitations on the format/surface/stride. Or it may not be desirable to use the specific GPU resource powering the "replaceable renderer". For instance for an extremely power efficient device operation, the replaceable renderer could be powered by a blitter, only on occasion falling back to 3D GPU. So the replaceable renderer could go either way - more fully fledged, or dumber. Whereas the hwc fallback renderer always have a very well-defined task to do.

[Kevin DuBois (kdub)]

So the specific thing that happened to me when I was developing was I was tinkering with the demo shell on the adreno nexus 7, and the driver optimized the render by clipping the pixels outside of the bounding box of the hwc_layer_1_t. (pushed out less pixels) Since the demo shell clears to grey, and the HWC api guarantees a black background, this made the image on the screen cleared to grey within the bounding box, and cleared to black outside the bounding box.

Now, this is 'valid' HWC behavior, as they only have to guarantee what was sent to the driver in prepare() makes it to the screen, but it is not valid OpenGL behavior.

Furthermore, an extra level of insidious-ness about this discrepancy is that only certain devices running under certain modes might step outside of the OpenGL box and rely on the less-universal requirements of HWC to try some optimization.

So, I'm more nervous about situations where doing something reasonable in GLRenderer (like setting a non-black background color) cause rendering glitches on specific devices, running under specific modes, only when the driver chooses to reject the overlay surface.

I'm less nervous about the task of setting up and maintaining a GL shader program that fulfills the requirements of HWC to the letter, and isolates GLRenderer/common gl code from the weirdness of HWC. The program will just do the basic stuff HWC needs (2D positioning, 90 degree rotation, cropping)

[Kevin DuBois (kdub)]

> +1 on the names Alberto is proposing.
>
> I think things will get interesting when YUV support is eventually added.
> Hopefully display overlay will support the format so hwc will not reject it.
> But if it does, it's almost certain that resize will be needed - video
> dimensions almost never match player's viewing area. Also, rotation should be
> supported IMHO, too.

I'm starting out with just the simple non-scaled, non-rotated, non-alpha blended stuff, and then I'll roll out alpha, and 90deg rotation, etc after some testing focused on those features. But yes, rotation is something that should be supported.

>
> Why not use the "replaceable renderer"? (I'm not calling it GLRenderer on
> purpose). It may not be capable of handling YUV due to, perhaps, hardware
> limitations on the format/surface/stride. Or it may not be desirable to use
> the specific GPU resource powering the "replaceable renderer". For instance
> for an extremely power efficient device operation, the replaceable renderer
> could be powered by a blitter, only on occasion falling back to 3D GPU. So the
> replaceable renderer could go either way - more fully fledged, or dumber.
> Whereas the hwc fallback renderer always have a very well-defined task to do.

YUV is interesting, and I saw that it got much more public support in HWC 1.3. (previously there were hacks behind the scenes iirc). It might be another place where we might have to mix some 'android-standard' approaches with the 'universal OpenGL' approach. There is a fair amount of plumbing in mir (libmirserver and libmirclient) that we have to do before YUV surfaces can get to the compositor though.

[Alberto Aguirre (albaguirre)]

> I'm starting out with just the simple non-scaled, non-rotated, non-alpha
> blended stuff, and then I'll roll out alpha, and 90deg rotation, etc after
> some testing focused on those features. But yes, rotation is something that
> should be supported.
>

That's my point though, if you punt to the GLRenderer (for ALL layers even if only one is rejected)
then there's no need to implement those features and the HWC Fallback renderer can remain very simple.

[Alberto Aguirre (albaguirre)]

> > I'm starting out with just the simple non-scaled, non-rotated, non-alpha
> > blended stuff, and then I'll roll out alpha, and 90deg rotation, etc after
> > some testing focused on those features. But yes, rotation is something that
> > should be supported.
> >
>
> That's my point though, if you punt to the GLRenderer (for ALL layers even if
> only one is rejected)
> then there's no need to implement those features and the HWC Fallback renderer
> can remain very simple.

I mean punt when layers are rejected and any of the rejected layers needs resizing, rotation etc...

[Kevin DuBois (kdub)]

so hopefully the naming should be addressed. "OverlayCompositor" was just the name of the test. The class and the test are now "HWCFallbackGLRenderer", as matching the class name to the test name seems less confusing/more consistent.

Still working on addressing the outstanding points, probably won't get them out the door today(4/30)

[Alberto Aguirre (albaguirre)]

In any case this first simple version looks good to me.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1585
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1456/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1782
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1780
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1349/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1188
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1188/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1193
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1193/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1352
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1352/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1245/console
    FAILURE: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6601/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1456/rebuild

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

Even if it is justified in introducing a new code path, I think we can still do a bit better in avoiding the very familiar duplication inside mga::HWCFallbackGLRenderer::render().

The first part of that (which I had hoped to do anyway) is to pass Renderer::render() the full list. Then the various render() functions start to look even more similar. And as such it will become easier to avoid duplicate code.

I'm not quite convinced enough yet. Because I think there is simple work we can do relatively quickly to reduce the duplication (and ideally avoid regressions landing).

[Kevin DuBois (kdub)]

> Even if it is justified in introducing a new code path, I think we can still
> do a bit better in avoiding the very familiar duplication inside
> mga::HWCFallbackGLRenderer::render().
>
> The first part of that (which I had hoped to do anyway) is to pass
> Renderer::render() the full list. Then the various render() functions start to
> look even more similar. And as such it will become easier to avoid duplicate
> code.

Given my thoughts about the quirks of HWC/OpenGL interplay, some object in the system must be the hardware-specific HWC renderer. It must play by the OpenGL+HWC limitations. There exists some other item in the system that can do full opengl, that the user can override, and can expand in whatever way we see fit. So the unresolved question is "is this second item a private instance of the GLRenderer?"

I would still say there's a good case to have something different:

1) although I have in rev1585:
void mga::HWCFallbackGLRenderer::render(RenderableList const& renderlist, SwappingGLContext const& context)
this really should be render(hwc_display_contents_1_t const&, SwappingGLContext). The Renderable's are just used now because I haven't made a way for the HWCLayers to return their associated buffers yet. Its convenient, albeit a bit fragile because the implementation has to use only the 'hwc-compatible' functions on Renderable.
1a) A Renderable is a superset of the information that a hwc_layer_1_t is. We could limit the GLRenderer to what the hwc_layer_1_t can do, or we can have the GLRenderer do more than what the hwc requires. It seems better to just let them evolve on their own path, especially as one requirement is platform-specific.

2) The requirement that the rejected overlays should be drawn via OpenGL is a quirk of the Android platform. Having a non-overridable OpenGL+HWC-restricted program in the platform-independent server code breaks the platform-independence in a worse way than allowing the platforms to compile their own gl programs. The platform independent libmirserver code has to respect specific guidelines set by the android platform, even if those requirements are tangled up in OpenGL.

3) In the same sort of vein, the object that supports this android quirk, and the object that is the platform-independent default renderer will take different paths. The have a fair amount of overlap in their current states, but we really do intend divergence in their responsibilities. The android stuff will evolve into deeper support of the lower-level optimizations (like, 'flash GL regions", YUV/color processing) and the platform-independent default renderer will evolve to support advanced shell stuff (animations, blurs, etc etc).

4) Splitting the two also lets us keep the interesting GLRenderer stuff GPL and the simplistic android stuff LGPL. Without getting too a head of myself, this could ease working with vendors when/if we come hwc extension agreements after working with them.

Now, having said all that :) The opposing point about code reuse is valid, and I can share some of the code by teasing it out of GLRenderer. I'm envisioning a smallish LGPL header with ways to do 1) texture caching 2) tessellation 3) texcood c...

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1585
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1483/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1814
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1812
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1381
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1215
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1215/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1220
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1220/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1384
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1384/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1277
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6666

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1483/rebuild

review: Approve (continuous-integration)

[Cemil Azizoglu (cemil-azizoglu)]

Happy with the names.

review: Approve

[Daniel van Vugt (vanvugt)]

(1) Still too much redundancy. And I don't think designing in regressions is OK, unless we agree no one is ever going to use the overlays path. We don't have to resolve this by inheriting from or calling GLRenderer necessarily. The various *GLRenderer's could call some common code that contains the shared logic.

(3) This class name is too long and also misleading - SingleVertexSingleFragmentProgram. A "single vertex" or "single fragment" program sounds too much like something designed to render 1x1 pixel points only, which is obviously untrue. Rendering anything that's not a point of course requires multiple vertices and many fragments (which is actually what it does). Something like "SimpleGLProgram" would be more accurate.

(4) Can you tease apart the proposal into smaller chunks? I suspect that might make it easier to manage...

review: Needs Fixing

[Kevin DuBois (kdub)]

2)
Sure, can address the naming. OneVertexShaderOneFragmentShaderProgram is too long, so SimpleGLProgram is okay.

1)
I haven't had as much time to work on splitting out the code this week as I'd have liked, but I do plan on reusing some of the various GLRenderer calls. Specifically, the texture caching, and the texcoord calculation. I can get these out the door before this lands so the reuse plan is more visible.

3)
I was thinking that anything less would might be hard to review from "dont see the big picture" argument.
This chunk of code is what I deemed the minimal amount that will get something sensible in this place, and less code would be difficult to figure out what I'm trying to do. Maybe I can break out the changes to the Rectangle eg, top_right(), or the interface changes (which were largely test-driven)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1586
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1507/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/32
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/33
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/33
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/26
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/26/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/24
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/24/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/39
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/39/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1303
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6744

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1507/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

> I haven't had as much time to work on splitting out the code this week as I'd
> have liked, but I do plan on reusing some of the various GLRenderer calls.
> Specifically, the texture caching, and the texcoord calculation. I can get
> these out the door before this lands so the reuse plan is more visible.

I still want to chew on the naming before mp-ing, but split out the texture caching here: lp:~kdub/mir/reusable-texture-cache. Will get to the texcoord stuff tomorrow, and rebase on those.

[Kevin DuBois (kdub)]

There's also lp:~kdub/mir/reusable-quad-calculation that this branch can pick up to increase the amount of code reuse between the two programs. Hopefully those two branches address some of the reused code concerns.

[Kevin DuBois (kdub)]

Since this is now based on two branches resubmitting with a prerequisite branch would still give too big of a diff. Maintaining the merge of the branches here: lp:~kdub/mir/overlay-gl-program-with-reuse , which I will merge into this branch once the prereqs land.

[Kevin DuBois (kdub)]

diff should reduce once the landing queue clears

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1606
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1699/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/289
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/290
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/289
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/218
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/218/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/217
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/217/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/770
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/770/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1535
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/7482

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1699/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1608
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1701/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/291
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/292
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/291
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/220
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/220/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/219
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/219/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/773
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/773/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1537
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/7486

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1701/rebuild

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Oh, good work keeping this branch up to date with the other landings.

I note that the current Approvals however are based on a much older and significantly version. So I think the current Approvals need to be invalidated by resubmitting the proposal.

review: Needs Resubmitting

[Alan Griffiths (alan-griffiths)]

151 + return 7;

Magic?

~~~~

790 +// EXPECT_CALL(mock_gl, glGenTextures(1,_));

Useless code or useless comment - either way it is useless.

review: Needs Fixing

[Kevin DuBois (kdub)]

@Daniel,
resubmitted.

@Alan,
not a magic number, just a valid number that the stub could return.
comment removed.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1610
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1713/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/309
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/310
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/309
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/232
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/232/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/231
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/231/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/834
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/834/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1553
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/7547

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1713/rebuild

review: Approve (continuous-integration)

[Alberto Aguirre (albaguirre)]

~~
151 + return 7;
~~
Perhaps:
int const arbitrary_id = 7;
return arbitrary_id;

Looks good.

review: Approve

[Cemil Azizoglu (cemil-azizoglu)]

LGTM

review: Approve

[Alan Griffiths (alan-griffiths)]

OK

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'include/platform/mir/graphics/gl_program.h'
--- include/platform/mir/graphics/gl_program.h	2014-05-07 02:56:33 +0000
+++ include/platform/mir/graphics/gl_program.h	2014-05-22 16:51:59 +0000
@@ -43,17 +43,26 @@
 class GLProgram
 {
 public:
-    GLProgram(
+    virtual ~GLProgram() = default;
+    virtual operator GLuint() const = 0;
+
+protected:
+    GLProgram() = default;
+private:
+    GLProgram(GLProgram const&) = delete;
+    GLProgram& operator=(GLProgram const&) = delete;
+};
+
+class SimpleGLProgram : public GLProgram
+{
+public:
+    SimpleGLProgram(
         GLchar const* vertex_shader_src,
         GLchar const* fragment_shader_src);
-    ~GLProgram();
-
-    operator GLuint() const;
-
+    ~SimpleGLProgram();
+
+    operator GLuint() const override;
 private:
-    GLProgram(GLProgram const&) = delete;
-    GLProgram& operator=(GLProgram const&) = delete;
-
     GLShader const vertex_shader; 
     GLShader const fragment_shader;
     GLuint const program;

=== modified file 'include/platform/mir/graphics/gl_program_factory.h'
--- include/platform/mir/graphics/gl_program_factory.h	2014-04-22 16:45:27 +0000
+++ include/platform/mir/graphics/gl_program_factory.h	2014-05-22 16:51:59 +0000
@@ -20,6 +20,7 @@
 #define MIR_COMPOSITOR_GL_PROGRAM_FACTORY_H_
 
 #include "mir/graphics/gl_program.h"
+#include "mir/graphics/gl_texture_cache.h"
 #include <memory>
 
 namespace mir
@@ -34,7 +35,7 @@
 
     virtual std::unique_ptr<GLProgram>
         create_gl_program(std::string const&, std::string const&) const = 0;
-
+    virtual std::unique_ptr<GLTextureCache> create_texture_cache() const = 0;
 protected:
     GLProgramFactory() = default;
 

=== modified file 'include/shared/mir/geometry/point.h'
--- include/shared/mir/geometry/point.h	2014-03-06 06:05:17 +0000
+++ include/shared/mir/geometry/point.h	2014-05-22 16:51:59 +0000
@@ -50,6 +50,9 @@
     return lhs.x != rhs.x || lhs.y != rhs.y;
 }
 
+inline Point operator+(Point lhs, DeltaX rhs) { return{lhs.x + rhs, lhs.y}; }
+inline Point operator+(Point lhs, DeltaY rhs) { return{lhs.x, lhs.y + rhs}; }
+
 std::ostream& operator<<(std::ostream& out, Point const& value);
 }
 }

=== modified file 'include/shared/mir/geometry/rectangle.h'
--- include/shared/mir/geometry/rectangle.h	2014-03-06 06:05:17 +0000
+++ include/shared/mir/geometry/rectangle.h	2014-05-22 16:51:59 +0000
@@ -42,6 +42,8 @@
      * area, that is, the rectangle is represented as [top_left,bottom_right).
      */
     Point bottom_right() const;
+    Point top_right() const;
+    Point bottom_left() const;
     bool contains(Point const& p) const;
 
     /**

=== modified file 'include/test/mir_test_doubles/mock_display_device.h'
--- include/test/mir_test_doubles/mock_display_device.h	2014-05-19 02:55:29 +0000
+++ include/test/mir_test_doubles/mock_display_device.h	2014-05-22 16:51:59 +0000
@@ -22,7 +22,7 @@
 #include "mir/graphics/buffer.h"
 #include "src/platform/graphics/android/display_device.h"
 #include "src/platform/graphics/android/gl_context.h"
-#include "src/platform/graphics/android/overlay_gl_compositor.h"
+#include "src/platform/graphics/android/hwc_fallback_gl_renderer.h"
 #include <gmock/gmock.h>
 
 namespace mir

=== modified file 'include/test/mir_test_doubles/mock_renderable_list_compositor.h'
--- include/test/mir_test_doubles/mock_renderable_list_compositor.h	2014-04-30 18:26:29 +0000
+++ include/test/mir_test_doubles/mock_renderable_list_compositor.h	2014-05-22 16:51:59 +0000
@@ -19,7 +19,7 @@
 #ifndef MIR_TEST_DOUBLES_MOCK_RENDERABLE_LIST_COMPOSITOR_H_
 #define MIR_TEST_DOUBLES_MOCK_RENDERABLE_LIST_COMPOSITOR_H_
 
-#include "src/platform/graphics/android/overlay_gl_compositor.h"
+#include "src/platform/graphics/android/hwc_fallback_gl_renderer.h"
 #include <gmock/gmock.h>
 
 namespace mir

=== added file 'include/test/mir_test_doubles/stub_gl_program.h'
--- include/test/mir_test_doubles/stub_gl_program.h	1970-01-01 00:00:00 +0000
+++ include/test/mir_test_doubles/stub_gl_program.h	2014-05-22 16:51:59 +0000
@@ -0,0 +1,42 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Kevin DuBois <[e-mail]>
+ */
+#ifndef MIR_TEST_DOUBLES_STUB_GL_PROGRAM_H_
+#define MIR_TEST_DOUBLES_STUB_GL_PROGRAM_H_
+
+#include "mir/graphics/gl_program_factory.h"
+
+namespace mir
+{
+namespace test
+{
+namespace doubles
+{
+
+struct StubGLProgram : public graphics::GLProgram
+{
+    operator GLuint() const override
+    {
+        return 7;
+    }
+};
+
+}
+}
+} // namespace mir
+
+#endif /* MIR_TEST_DOUBLES_STUB_GL_PROGRAM_H_ */

=== modified file 'include/test/mir_test_doubles/stub_gl_program_factory.h'
--- include/test/mir_test_doubles/stub_gl_program_factory.h	2014-04-24 20:07:20 +0000
+++ include/test/mir_test_doubles/stub_gl_program_factory.h	2014-05-22 16:51:59 +0000
@@ -19,6 +19,7 @@
 #define MIR_TEST_DOUBLES_STUB_GL_PROGRAM_FACTORY_H_
 
 #include "mir/graphics/gl_program_factory.h"
+#include "stub_gl_program.h"
 
 namespace mir
 {
@@ -32,6 +33,11 @@
 public:
     std::unique_ptr<graphics::GLProgram> create_gl_program(std::string const&, std::string const&) const
     {
+        return std::unique_ptr<graphics::GLProgram>(new StubGLProgram);
+    }
+
+    std::unique_ptr<graphics::GLTextureCache> create_texture_cache() const
+    {
         return nullptr;
     }
 };

=== modified file 'include/test/mir_test_doubles/stub_renderable.h'
--- include/test/mir_test_doubles/stub_renderable.h	2014-05-07 02:56:33 +0000
+++ include/test/mir_test_doubles/stub_renderable.h	2014-05-22 16:51:59 +0000
@@ -33,13 +33,20 @@
 class StubRenderable : public graphics::Renderable
 {
 public:
+    StubRenderable(geometry::Rectangle const& rect)
+        : rect(rect)
+    {}
+    StubRenderable() :
+        StubRenderable({{},{}})
+    {}
     ID id() const override
     {
         return this;
     }
     std::shared_ptr<graphics::Buffer> buffer() const override
     {
-        return std::make_shared<StubBuffer>();
+        graphics::BufferProperties prop{rect.size, mir_pixel_format_abgr_8888, graphics::BufferUsage::hardware};
+        return std::make_shared<StubBuffer>(prop);
     }
     bool alpha_enabled() const
     {
@@ -47,7 +54,7 @@
     }
     geometry::Rectangle screen_position() const
     {
-        return {{},{}};
+        return rect;
     }
     float alpha() const override
     {
@@ -73,6 +80,7 @@
 
 private:
     glm::mat4 trans;
+    geometry::Rectangle const rect;
 };
 
 }

=== modified file 'include/test/mir_test_doubles/stub_renderable_list_compositor.h'
--- include/test/mir_test_doubles/stub_renderable_list_compositor.h	2014-04-30 18:26:29 +0000
+++ include/test/mir_test_doubles/stub_renderable_list_compositor.h	2014-05-22 16:51:59 +0000
@@ -19,7 +19,7 @@
 #ifndef MIR_TEST_DOUBLES_STUB_RENDERABLE_LIST_COMPOSITOR_H_
 #define MIR_TEST_DOUBLES_STUB_RENDERABLE_LIST_COMPOSITOR_H_
 
-#include "src/platform/graphics/android/overlay_gl_compositor.h"
+#include "src/platform/graphics/android/hwc_fallback_gl_renderer.h"
 
 namespace mir
 {

=== modified file 'src/platform/graphics/android/CMakeLists.txt'
--- src/platform/graphics/android/CMakeLists.txt	2014-05-22 11:31:21 +0000
+++ src/platform/graphics/android/CMakeLists.txt	2014-05-22 16:51:59 +0000
@@ -36,7 +36,7 @@
   gl_context.cpp
   device_quirks.cpp
   real_hwc_wrapper.cpp
-  overlay_gl_compositor.cpp
+  hwc_fallback_gl_renderer.cpp
 )
 
 set_target_properties(

=== modified file 'src/platform/graphics/android/display_buffer.cpp'
--- src/platform/graphics/android/display_buffer.cpp	2014-05-22 11:31:21 +0000
+++ src/platform/graphics/android/display_buffer.cpp	2014-05-22 16:51:59 +0000
@@ -62,7 +62,7 @@
       display_device{display_device},
       native_window{native_window},
       gl_context{shared_gl_context, std::bind(mga::create_window_surface, std::placeholders::_1, std::placeholders::_2, native_window.get())},
-      overlay_program{program_factory, gl_context},
+      overlay_program{program_factory, gl_context, geom::Rectangle{{0,0},fb_bundle->fb_size()}},
       current_configuration{
           mg::DisplayConfigurationOutputId{1},
           mg::DisplayConfigurationCardId{0},

=== modified file 'src/platform/graphics/android/display_buffer.h'
--- src/platform/graphics/android/display_buffer.h	2014-05-22 11:31:21 +0000
+++ src/platform/graphics/android/display_buffer.h	2014-05-22 16:51:59 +0000
@@ -24,7 +24,7 @@
 #include "mir/graphics/gl_program_factory.h"
 #include "android_display_configuration.h"
 #include "gl_context.h"
-#include "overlay_gl_compositor.h"
+#include "hwc_fallback_gl_renderer.h"
 #include <system/window.h>
 
 namespace mir
@@ -65,7 +65,7 @@
     std::shared_ptr<DisplayDevice> const display_device;
     std::shared_ptr<ANativeWindow> const native_window;
     GLContext gl_context;
-    OverlayGLProgram overlay_program;
+    HWCFallbackGLRenderer overlay_program;
     bool prepared;
     DisplayConfigurationOutput current_configuration;
     MirOrientation rotation;

=== modified file 'src/platform/graphics/android/fb_device.cpp'
--- src/platform/graphics/android/fb_device.cpp	2014-05-19 02:55:29 +0000
+++ src/platform/graphics/android/fb_device.cpp	2014-05-22 16:51:59 +0000
@@ -24,7 +24,7 @@
 #include "fb_device.h"
 #include "framebuffer_bundle.h"
 #include "buffer.h"
-#include "overlay_gl_compositor.h"
+#include "hwc_fallback_gl_renderer.h"
 
 #include <boost/throw_exception.hpp>
 #include <stdexcept>

=== modified file 'src/platform/graphics/android/hwc_device.cpp'
--- src/platform/graphics/android/hwc_device.cpp	2014-05-19 02:55:29 +0000
+++ src/platform/graphics/android/hwc_device.cpp	2014-05-22 16:51:59 +0000
@@ -24,7 +24,7 @@
 #include "hwc_wrapper.h"
 #include "framebuffer_bundle.h"
 #include "buffer.h"
-#include "overlay_gl_compositor.h"
+#include "hwc_fallback_gl_renderer.h"
 
 namespace mg = mir::graphics;
 namespace mga=mir::graphics::android;

=== renamed file 'src/platform/graphics/android/overlay_gl_compositor.cpp' => 'src/platform/graphics/android/hwc_fallback_gl_renderer.cpp'
--- src/platform/graphics/android/overlay_gl_compositor.cpp	2014-05-19 02:55:29 +0000
+++ src/platform/graphics/android/hwc_fallback_gl_renderer.cpp	2014-05-22 16:51:59 +0000
@@ -18,36 +18,113 @@
 
 #include "mir/graphics/gl_program_factory.h"
 #include "mir/graphics/gl_context.h"
-#include "overlay_gl_compositor.h"
+#include "mir/graphics/gl_texture.h"
+#include "mir/graphics/tessellation_helpers.h"
+#include "hwc_fallback_gl_renderer.h"
+#include "gl_context.h"
+#include "buffer.h"
+
+#define GLM_FORCE_RADIANS
+#define GLM_PRECISION_MEDIUMP_FLOAT
+#include <glm/glm.hpp>
+#include <glm/gtc/matrix_transform.hpp>
+#include <glm/gtc/type_ptr.hpp>
 
 namespace mg = mir::graphics;
 namespace mga = mir::graphics::android;
+namespace geom = mir::geometry;
+
 namespace
 {
 std::string const vertex_shader
 {
-    "attribute vec4 position;\n"
+    "attribute vec3 position;\n"
+    "attribute vec2 texcoord;\n"
+    "uniform mat4 display_transform;\n"
+    "varying vec2 v_texcoord;\n"
     "void main() {\n"
-    "   gl_Position = position;\n"
+    "   gl_Position = display_transform * vec4(position, 1.0);\n"
+    "   v_texcoord = texcoord;\n"
     "}\n"
 };
 
 std::string const fragment_shader
 {
     "precision mediump float;\n"
+    "uniform sampler2D tex;\n"
+    "varying vec2 v_texcoord;\n"
     "void main() {\n"
-    "   gl_FragColor = vec4(1.0, 1.0, 1.0, 1.0);\n"
+    "   gl_FragColor = texture2D(tex, v_texcoord);\n"
     "}\n"
 };
-}
-mga::OverlayGLProgram::OverlayGLProgram(
-    GLProgramFactory const& factory, GLContext const& context)
+
+void set_display_transform(GLint uniform_loc, geom::Rectangle const& rect)
+{
+    glm::mat4 disp_transform(1.0);
+
+    disp_transform = glm::translate(disp_transform, glm::vec3{-1.0, 1.0, 0.0});
+    disp_transform = glm::scale(
+                        disp_transform,
+                        glm::vec3{2.0/rect.size.width.as_float(),
+                                  -2.0/rect.size.height.as_float(),
+                                  1.0});
+    glUniformMatrix4fv(uniform_loc, 1, GL_FALSE, glm::value_ptr(disp_transform));
+}
+}
+
+mga::HWCFallbackGLRenderer::HWCFallbackGLRenderer(
+    GLProgramFactory const& factory,
+    mg::GLContext const& context,
+    geom::Rectangle const& screen_pos)
 {
     context.make_current();
-    overlay_program = factory.create_gl_program(vertex_shader, fragment_shader);
+    program = factory.create_gl_program(vertex_shader, fragment_shader);
+    texture_cache = factory.create_texture_cache();
+
+    glUseProgram(*program);
+
+    auto display_transform_uniform = glGetUniformLocation(*program, "display_transform");
+    set_display_transform(display_transform_uniform, screen_pos); 
+
+    position_attr = glGetAttribLocation(*program, "position");
+    texcoord_attr = glGetAttribLocation(*program, "texcoord");
+
+    auto tex_loc = glGetUniformLocation(*program, "tex");
+    glUniform1i(tex_loc, 0);
+
+    glBindBuffer(GL_ARRAY_BUFFER, 0);
+
+    glUseProgram(0);
     context.release_current();
 }
 
-void mga::OverlayGLProgram::render(RenderableList const&, SwappingGLContext const&) const
+void mga::HWCFallbackGLRenderer::render(
+    RenderableList const& renderlist, SwappingGLContext const& context) const
 {
+    glUseProgram(*program);
+
+    glClearColor(0.0, 0.0, 0.0, 1.0);
+    glClear(GL_COLOR_BUFFER_BIT);
+    
+    glEnableVertexAttribArray(position_attr);
+    glEnableVertexAttribArray(texcoord_attr);
+
+    for(auto const& renderable : renderlist)
+    {
+        auto const primitive = mg::tessellate_renderable_into_rectangle(*renderable);
+        glVertexAttribPointer(position_attr, 3, GL_FLOAT, GL_FALSE, sizeof(mg::GLVertex),
+                              &primitive.vertices[0].position);
+        //TODO: (kdub) scaling or pi/2 rotation eventually. for now, all quads get same texcoords
+        glVertexAttribPointer(texcoord_attr, 2, GL_FLOAT, GL_FALSE, sizeof(mg::GLVertex),
+                              &primitive.vertices[0].texcoord);
+
+        texture_cache->load(*renderable)->bind();
+        glDrawArrays(primitive.type, 0, primitive.vertices.size());
+    }
+
+    glDisableVertexAttribArray(texcoord_attr);
+    glDisableVertexAttribArray(position_attr);
+    context.swap_buffers();
+    texture_cache->drop_unused();
+    glUseProgram(0);
 }

=== renamed file 'src/platform/graphics/android/overlay_gl_compositor.h' => 'src/platform/graphics/android/hwc_fallback_gl_renderer.h'
--- src/platform/graphics/android/overlay_gl_compositor.h	2014-05-19 02:55:29 +0000
+++ src/platform/graphics/android/hwc_fallback_gl_renderer.h	2014-05-22 16:51:59 +0000
@@ -16,11 +16,12 @@
  * Authored by: Kevin DuBois <[e-mail]>
  */
 
-#ifndef MIR_GRAPHICS_ANDROID_OVERLAY_GL_PROGRAM_H_
-#define MIR_GRAPHICS_ANDROID_OVERLAY_GL_PROGRAM_H_
-
+#ifndef MIR_GRAPHICS_ANDROID_HWC_FALLBACK_GL_RENDERER_H_
+#define MIR_GRAPHICS_ANDROID_HWC_FALLBACK_GL_RENDERER_H_
+#include "mir/geometry/rectangle.h"
 #include "mir/graphics/gl_program.h"
 #include "mir/graphics/renderable.h"
+#include "mir/graphics/gl_texture_cache.h"
 #include <memory>
 
 namespace mir
@@ -46,18 +47,25 @@
     RenderableListCompositor& operator=(RenderableListCompositor const&) = delete;
 };
 
-class OverlayGLProgram : public RenderableListCompositor
+class HWCFallbackGLRenderer : public RenderableListCompositor
 {
 public:
-    OverlayGLProgram(GLProgramFactory const& program_factory, graphics::GLContext const& gl_context);
+    HWCFallbackGLRenderer(
+        GLProgramFactory const& program_factory,
+        graphics::GLContext const& gl_context,
+        geometry::Rectangle const& screen_position);
 
     void render(RenderableList const&, SwappingGLContext const&) const;
 private:
-    std::unique_ptr<graphics::GLProgram> overlay_program;
+    std::unique_ptr<graphics::GLProgram> program;
+    std::unique_ptr<graphics::GLTextureCache> texture_cache;
+
+    GLint position_attr;
+    GLint texcoord_attr;
 };
 
 }
 }
 }
 
-#endif /* MIR_GRAPHICS_ANDROID_OVERLAY_GL_PROGRAM_H_ */
+#endif /* MIR_GRAPHICS_ANDROID_HWC_FALLBACK_GL_RENDERER_H_ */

=== modified file 'src/platform/graphics/android/hwc_fb_device.cpp'
--- src/platform/graphics/android/hwc_fb_device.cpp	2014-05-19 02:55:29 +0000
+++ src/platform/graphics/android/hwc_fb_device.cpp	2014-05-22 16:51:59 +0000
@@ -22,7 +22,7 @@
 #include "framebuffer_bundle.h"
 #include "android_format_conversion-inl.h"
 #include "hwc_wrapper.h"
-#include "overlay_gl_compositor.h"
+#include "hwc_fallback_gl_renderer.h"
 #include "mir/graphics/buffer.h"
 #include "mir/graphics/android/native_buffer.h"
 #include "gl_context.h"

=== modified file 'src/platform/graphics/gl_program.cpp'
--- src/platform/graphics/gl_program.cpp	2014-05-22 11:31:21 +0000
+++ src/platform/graphics/gl_program.cpp	2014-05-22 16:51:59 +0000
@@ -76,7 +76,7 @@
     return shader;
 }
 
-mg::GLProgram::GLProgram(
+mg::SimpleGLProgram::SimpleGLProgram(
     GLchar const* vertex_shader_src,
     GLchar const* fragment_shader_src)
   : vertex_shader(vertex_shader_src, GL_VERTEX_SHADER),
@@ -98,12 +98,12 @@
     }
 }
 
-mg::GLProgram::~GLProgram()
+mg::SimpleGLProgram::~SimpleGLProgram()
 {
     glDeleteProgram(program);
 }
 
-mg::GLProgram::operator GLuint() const
+mg::SimpleGLProgram::operator GLuint() const
 {
     return program;
 }

=== modified file 'src/server/compositor/gl_renderer.cpp'
--- src/server/compositor/gl_renderer.cpp	2014-05-22 11:31:21 +0000
+++ src/server/compositor/gl_renderer.cpp	2014-05-22 16:51:59 +0000
@@ -149,7 +149,7 @@
     tessellate(primitives, renderable);
 
     auto surface_tex = texture_cache->load(renderable);
-   
+
     for (auto const& p : primitives)
     {
         // Note a primitive tex_id of zero means use the surface texture,

=== modified file 'src/server/graphics/program_factory.cpp'
--- src/server/graphics/program_factory.cpp	2014-04-16 19:00:03 +0000
+++ src/server/graphics/program_factory.cpp	2014-05-22 16:51:59 +0000
@@ -19,6 +19,7 @@
 
 #include "program_factory.h"
 #include "mir/graphics/gl_program.h"
+#include "mir/compositor/recently_used_cache.h"
 
 namespace mg = mir::graphics;
 
@@ -28,5 +29,12 @@
     std::string const& fragment_shader) const
 {
     std::lock_guard<decltype(mutex)> lock(mutex);
-    return std::unique_ptr<mg::GLProgram>(new GLProgram(vertex_shader.c_str(), fragment_shader.c_str()));
+    return std::unique_ptr<mg::GLProgram>(
+        new SimpleGLProgram(vertex_shader.c_str(), fragment_shader.c_str()));
+}
+
+std::unique_ptr<mg::GLTextureCache> mg::ProgramFactory::create_texture_cache() const
+{
+    return std::unique_ptr<mg::GLTextureCache>(
+        new mir::compositor::RecentlyUsedCache());
 }

=== modified file 'src/server/graphics/program_factory.h'
--- src/server/graphics/program_factory.h	2014-04-16 19:00:03 +0000
+++ src/server/graphics/program_factory.h	2014-05-22 16:51:59 +0000
@@ -21,6 +21,7 @@
 
 #include "mir/graphics/gl_program_factory.h"
 #include "mir/graphics/gl_program.h"
+#include "mir/graphics/gl_texture_cache.h"
 #include <mutex>
 
 namespace mir
@@ -31,6 +32,7 @@
 {
 public:
     std::unique_ptr<GLProgram> create_gl_program(std::string const&, std::string const&) const override;
+    std::unique_ptr<GLTextureCache> create_texture_cache() const;
 
 private:
     /*

=== modified file 'src/shared/geometry/rectangle.cpp'
--- src/shared/geometry/rectangle.cpp	2013-10-15 08:53:10 +0000
+++ src/shared/geometry/rectangle.cpp	2014-05-22 16:51:59 +0000
@@ -28,6 +28,16 @@
             top_left.y.as_int() + size.height.as_int()};
 }
 
+geom::Point geom::Rectangle::top_right() const
+{
+    return top_left + DeltaX{size.width.as_int()};
+}
+
+geom::Point geom::Rectangle::bottom_left() const
+{
+    return top_left + DeltaY{size.height.as_int()};
+}
+
 bool geom::Rectangle::contains(Rectangle const& r) const
 {
     return r.top_left.x >= top_left.x &&

=== modified file 'tests/unit-tests/graphics/android/CMakeLists.txt'
--- tests/unit-tests/graphics/android/CMakeLists.txt	2014-05-22 11:31:21 +0000
+++ tests/unit-tests/graphics/android/CMakeLists.txt	2014-05-22 16:51:59 +0000
@@ -27,7 +27,7 @@
   ${CMAKE_CURRENT_SOURCE_DIR}/test_output_builder.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_device_detection.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_hwc_wrapper.cpp
-  ${CMAKE_CURRENT_SOURCE_DIR}/test_overlay_compositor.cpp
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_hwc_fallback_gl_renderer.cpp
 )
 
 set(UNIT_TEST_SOURCES ${UNIT_TEST_SOURCES} PARENT_SCOPE)

=== modified file 'tests/unit-tests/graphics/android/test_fb_device.cpp'
--- tests/unit-tests/graphics/android/test_fb_device.cpp	2014-05-21 17:18:51 +0000
+++ tests/unit-tests/graphics/android/test_fb_device.cpp	2014-05-22 16:51:59 +0000
@@ -19,7 +19,7 @@
 #include "mir_test_doubles/mock_fb_hal_device.h"
 #include "mir_test_doubles/mock_buffer.h"
 #include "src/platform/graphics/android/fb_device.h"
-#include "src/platform/graphics/android/overlay_gl_compositor.h"
+#include "src/platform/graphics/android/hwc_fallback_gl_renderer.h"
 #include "mir_test_doubles/mock_framebuffer_bundle.h"
 #include "mir_test_doubles/mock_android_hw.h"
 #include "mir_test_doubles/mock_egl.h"

=== modified file 'tests/unit-tests/graphics/android/test_hwc_display.cpp'
--- tests/unit-tests/graphics/android/test_hwc_display.cpp	2014-05-22 11:31:21 +0000
+++ tests/unit-tests/graphics/android/test_hwc_display.cpp	2014-05-22 16:51:59 +0000
@@ -24,6 +24,7 @@
 #include "mir_test_doubles/mock_display_report.h"
 #include "mir_test_doubles/stub_renderable.h"
 #include "mir_test_doubles/mock_egl.h"
+#include "mir_test_doubles/mock_gl.h"
 #include "mir/graphics/android/mir_native_window.h"
 #include "mir_test_doubles/stub_driver_interpreter.h"
 #include "mir_test_doubles/stub_display_buffer.h"
@@ -103,6 +104,7 @@
     }
 
     testing::NiceMock<mtd::MockEGL> mock_egl;
+    testing::NiceMock<mtd::MockGL> mock_gl;
     mtd::StubGLProgramFactory stub_program_factory;
 
     int visual_id;

=== renamed file 'tests/unit-tests/graphics/android/test_overlay_compositor.cpp' => 'tests/unit-tests/graphics/android/test_hwc_fallback_gl_renderer.cpp'
--- tests/unit-tests/graphics/android/test_overlay_compositor.cpp	2014-04-24 20:07:20 +0000
+++ tests/unit-tests/graphics/android/test_hwc_fallback_gl_renderer.cpp	2014-05-22 16:51:59 +0000
@@ -16,18 +16,30 @@
  * Authored by: Kevin DuBois <[e-mail]>
  */
 
-#include "src/platform/graphics/android/overlay_gl_compositor.h"
+#include "src/platform/graphics/android/hwc_fallback_gl_renderer.h"
 #include "mir/graphics/gl_context.h"
 #include "mir/graphics/gl_program_factory.h"
+#include "mir/graphics/gl_primitive.h"
+#include "mir/graphics/gl_texture.h"
 #include "mir_test_doubles/mock_gl.h"
+#include "mir_test_doubles/mock_egl.h"
+#include "mir_test_doubles/stub_renderable.h"
+#include "mir_test_doubles/mock_swapping_gl_context.h"
+#include "mir_test_doubles/stub_gl_program.h"
 #include <gtest/gtest.h>
 #include <mir_test/gmock_fixes.h>
 
+#define GLM_FORCE_RADIANS
+#define GLM_PRECISION_MEDIUMP_FLOAT
+#include <glm/glm.hpp>
+#include <glm/gtc/matrix_transform.hpp>
+#include <glm/gtc/type_ptr.hpp>
+
 namespace mg=mir::graphics;
 namespace mga=mir::graphics::android;
 namespace mt=mir::test;
 namespace mtd=mir::test::doubles;
-
+namespace geom=mir::geometry;
 namespace
 {
 
@@ -36,6 +48,7 @@
 public:
     MOCK_CONST_METHOD2(create_gl_program,
         std::unique_ptr<mg::GLProgram>(std::string const&, std::string const&));
+    MOCK_CONST_METHOD0(create_texture_cache, std::unique_ptr<mg::GLTextureCache>());
 };
 
 class MockContext : public mg::GLContext
@@ -45,23 +58,258 @@
     MOCK_CONST_METHOD0(release_current, void());
 };
 
-class OverlayCompositor : public ::testing::Test
+struct MockTextureCache : public mg::GLTextureCache
+{
+    MockTextureCache()
+    {
+        ON_CALL(*this, load(testing::_))
+            .WillByDefault(testing::Return(std::make_shared<mg::GLTexture>()));
+    }
+    MOCK_METHOD1(load, std::shared_ptr<mg::GLTexture>(mg::Renderable const&));
+    MOCK_METHOD0(invalidate, void());
+    MOCK_METHOD0(drop_unused, void());
+};
+
+class StubTextureCache : public mg::GLTextureCache
+{
+    std::shared_ptr<mg::GLTexture> load(mg::Renderable const&)
+    {
+        return std::make_shared<mg::GLTexture>();
+    }
+    void invalidate()
+    {
+    }
+    void drop_unused()
+    {
+    }
+};
+
+class HWCFallbackGLRenderer : public ::testing::Test
 {
 public:
+    HWCFallbackGLRenderer()
+    {
+        using namespace testing;
+        ON_CALL(mock_gl_program_factory,create_gl_program(_,_))
+            .WillByDefault(Invoke([](std::string const, std::string const)
+                { return std::unique_ptr<mg::GLProgram>(new mtd::StubGLProgram); }));
+        ON_CALL(mock_gl_program_factory,create_texture_cache())
+            .WillByDefault(Invoke([]()
+                { return std::unique_ptr<mg::GLTextureCache>(new StubTextureCache); }));
+
+        ON_CALL(mock_gl, glGetShaderiv(_,_,_))
+            .WillByDefault(SetArgPointee<2>(GL_TRUE));
+        ON_CALL(mock_gl, glGetProgramiv(_,_,_))
+            .WillByDefault(SetArgPointee<2>(GL_TRUE));
+        ON_CALL(mock_gl, glGetUniformLocation(_, StrEq("display_transform")))
+            .WillByDefault(Return(display_transform_uniform_loc));
+        ON_CALL(mock_gl, glGetUniformLocation(_, StrEq("tex")))
+            .WillByDefault(Return(tex_uniform_loc));
+        ON_CALL(mock_gl, glGetAttribLocation(_, StrEq("position")))
+            .WillByDefault(Return(position_attr_loc));
+        ON_CALL(mock_gl, glGetAttribLocation(_, StrEq("texcoord")))
+            .WillByDefault(Return(texcoord_attr_loc));
+        ON_CALL(mock_gl, glGenTextures(1,_))
+            .WillByDefault(SetArgPointee<1>(texid));
+    }
+
+    GLint const display_transform_uniform_loc{1};
+    GLint const position_attr_loc{2};
+    GLint const texcoord_attr_loc{3};
+    GLint const tex_uniform_loc{4};
+    GLint const texid{5};
+    size_t const stride{sizeof(mg::GLVertex)};
+
     testing::NiceMock<MockGLProgramFactory> mock_gl_program_factory;
     testing::NiceMock<MockContext> mock_context;
     testing::NiceMock<mtd::MockGL> mock_gl;
+    testing::NiceMock<mtd::MockEGL> mock_egl;
+    geom::Rectangle dummy_screen_pos{geom::Point{0,0}, geom::Size{500,400}};
 };
-
 }
 
-TEST_F(OverlayCompositor, compiles_in_constructor)
+TEST_F(HWCFallbackGLRenderer, compiles_and_sets_up_gl_program)
 {
     using namespace testing;
     InSequence seq;
     EXPECT_CALL(mock_context, make_current());
     EXPECT_CALL(mock_gl_program_factory, create_gl_program(_,_));
+    EXPECT_CALL(mock_gl, glUseProgram(_));
+    EXPECT_CALL(mock_gl, glGetUniformLocation(_, StrEq("display_transform")));
+    EXPECT_CALL(mock_gl, glGetAttribLocation(_, StrEq("position")));
+    EXPECT_CALL(mock_gl, glGetAttribLocation(_, StrEq("texcoord")));
+    EXPECT_CALL(mock_gl, glGetUniformLocation(_, StrEq("tex")));
+    EXPECT_CALL(mock_gl, glUniform1i(tex_uniform_loc, 0));
+    EXPECT_CALL(mock_gl, glUseProgram(0));
     EXPECT_CALL(mock_context, release_current());
 
-    mga::OverlayGLProgram glprogram(mock_gl_program_factory, mock_context);
+    mga::HWCFallbackGLRenderer glprogram(mock_gl_program_factory, mock_context, dummy_screen_pos);
+}
+
+MATCHER_P(Matches4x4Matrix, value, "matches expected 4x4 matrix")
+{
+    for(auto i=0; i < 16; i++)
+        EXPECT_THAT(arg[i], testing::FloatEq(value[i]));
+    return !(::testing::Test::HasFailure());
+}
+
+TEST_F(HWCFallbackGLRenderer, sets_up_orthographic_matrix_based_on_screen_size)
+{
+    using namespace testing;
+    geom::Size sz{800,600};
+    geom::Point pt{100,200};
+    geom::Rectangle screen_pos{pt, sz};
+    float inv_w = 2.0/sz.width.as_int();
+    float inv_h = 2.0/sz.height.as_int() * -1.0;
+
+    float expected_matrix[]{
+        inv_w, 0.0    , 0.0, 0.0,
+        0.0  , inv_h  , 0.0, 0.0,
+        0.0  , 0.0    , 1.0, 0.0,
+        -1.0 , 1.0    , 0.0, 1.0
+    };
+
+    EXPECT_CALL(mock_gl, glUniformMatrix4fv(
+        display_transform_uniform_loc, 1, GL_FALSE, Matches4x4Matrix(expected_matrix)));
+
+    mga::HWCFallbackGLRenderer glprogram(mock_gl_program_factory, mock_context, screen_pos);
+}
+
+struct Vertex 
+{
+    float x;
+    float y;
+};
+
+Vertex to_vertex(geom::Point const& pos)
+{
+    return {
+        static_cast<float>(pos.x.as_int()),
+        static_cast<float>(pos.y.as_int())
+    };
+}
+
+MATCHER_P2(MatchesVertices, vertices, stride, "matches vertices")
+{
+    auto arg_vertices = static_cast<char const*>(arg);
+    for(auto const& vert : vertices)
+    {
+        GLfloat *f = (GLfloat*) arg_vertices;
+        EXPECT_THAT(f[0], testing::FloatEq(vert.x));
+        EXPECT_THAT(f[1], testing::FloatEq(vert.y));
+        arg_vertices+=stride;
+    } 
+    return !(::testing::Test::HasFailure());
+}
+
+TEST_F(HWCFallbackGLRenderer, computes_vertex_coordinates_correctly)
+{
+    using namespace testing;
+    NiceMock<mtd::MockSwappingGLContext> mock_context_s;
+    geom::Rectangle rect1{{100,200},{50, 60}};
+    geom::Rectangle rect2{{150,250},{150, 90}};
+    
+    mg::RenderableList renderlist{
+        std::make_shared<mtd::StubRenderable>(rect1),
+        std::make_shared<mtd::StubRenderable>(rect2)
+    };
+
+    std::vector<Vertex> expected_vertices1 {
+        to_vertex(rect1.top_left),
+        to_vertex(rect1.bottom_left()),
+        to_vertex(rect1.top_right()),
+        to_vertex(rect1.bottom_right()),
+    };
+    std::vector<Vertex> expected_vertices2 {
+        to_vertex(rect2.top_left),
+        to_vertex(rect2.bottom_left()),
+        to_vertex(rect2.top_right()),
+        to_vertex(rect2.bottom_right()),
+    };
+
+    InSequence seq;
+    EXPECT_CALL(mock_gl, glVertexAttribPointer(
+        position_attr_loc, 3, GL_FLOAT, GL_FALSE, stride, MatchesVertices(expected_vertices1, stride)));
+    EXPECT_CALL(mock_gl, glVertexAttribPointer(_,_,_,_,_,_))
+        .Times(1);
+    EXPECT_CALL(mock_gl, glVertexAttribPointer(
+        position_attr_loc, 3, GL_FLOAT, GL_FALSE, stride, MatchesVertices(expected_vertices2, stride)));
+    EXPECT_CALL(mock_gl, glVertexAttribPointer(_,_,_,_,_,_))
+        .Times(1);
+
+    mga::HWCFallbackGLRenderer glprogram(mock_gl_program_factory, mock_context, dummy_screen_pos);
+    glprogram.render(renderlist, mock_context_s);
+}
+
+TEST_F(HWCFallbackGLRenderer, computes_texture_coordinates_correctly)
+{
+    using namespace testing;
+    NiceMock<mtd::MockSwappingGLContext> mock_context_s;
+    geom::Rectangle rect1{{100,200},{50, 60}};
+    geom::Rectangle rect2{{150,250},{150, 90}};
+    
+    mg::RenderableList renderlist{
+        std::make_shared<mtd::StubRenderable>(rect1), 
+        std::make_shared<mtd::StubRenderable>(rect2)
+    };
+
+    std::vector<Vertex> expected_texcoord {
+        {0.0f, 0.0f},
+        {0.0f, 1.0f},
+        {1.0f, 0.0f},
+        {1.0f, 1.0f}
+    };
+
+    EXPECT_CALL(mock_gl, glVertexAttribPointer(_,_,_,_,_,_)).Times(AnyNumber());
+    EXPECT_CALL(mock_gl, glVertexAttribPointer(
+        texcoord_attr_loc, 2, GL_FLOAT, GL_FALSE, stride, MatchesVertices(expected_texcoord, stride)))
+        .Times(2);
+
+    mga::HWCFallbackGLRenderer glprogram(mock_gl_program_factory, mock_context, dummy_screen_pos);
+    glprogram.render(renderlist, mock_context_s);
+}
+
+TEST_F(HWCFallbackGLRenderer, executes_render_in_sequence)
+{
+    using namespace testing;
+    NiceMock<mtd::MockSwappingGLContext> mock_context_s;
+    auto renderable1 = std::make_shared<mtd::StubRenderable>();
+    auto renderable2 = std::make_shared<mtd::StubRenderable>();
+    mg::RenderableList renderlist{ renderable1, renderable2 };
+
+    std::unique_ptr<MockTextureCache> mock_texture_cache(new MockTextureCache);
+    {
+        InSequence seq;
+        EXPECT_CALL(*mock_texture_cache, load(Ref(*renderable1))); 
+        EXPECT_CALL(*mock_texture_cache, load(Ref(*renderable2))); 
+        EXPECT_CALL(*mock_texture_cache, drop_unused());
+    }
+    ON_CALL(mock_gl_program_factory,create_texture_cache())
+        .WillByDefault(Invoke([&](){
+                return std::move(mock_texture_cache);
+            }));
+
+    mga::HWCFallbackGLRenderer glprogram(mock_gl_program_factory, mock_context, dummy_screen_pos);
+
+    InSequence seq;
+    EXPECT_CALL(mock_gl, glUseProgram(_));
+    EXPECT_CALL(mock_gl, glClearColor(FloatEq(0.0), FloatEq(0.0), FloatEq(0.0), FloatEq(1.0)));
+    EXPECT_CALL(mock_gl, glClear(GL_COLOR_BUFFER_BIT));
+    EXPECT_CALL(mock_gl, glEnableVertexAttribArray(position_attr_loc));
+    EXPECT_CALL(mock_gl, glEnableVertexAttribArray(texcoord_attr_loc));
+
+    EXPECT_CALL(mock_gl, glVertexAttribPointer(position_attr_loc, 3, GL_FLOAT, GL_FALSE, _, _));
+    EXPECT_CALL(mock_gl, glVertexAttribPointer(texcoord_attr_loc, 2, GL_FLOAT, GL_FALSE, _, _));
+    EXPECT_CALL(mock_gl, glDrawArrays(GL_TRIANGLE_STRIP, 0, 4));
+
+    EXPECT_CALL(mock_gl, glVertexAttribPointer(position_attr_loc, 3, GL_FLOAT, GL_FALSE, _, _));
+    EXPECT_CALL(mock_gl, glVertexAttribPointer(texcoord_attr_loc, 2, GL_FLOAT, GL_FALSE, _, _));
+    EXPECT_CALL(mock_gl, glDrawArrays(GL_TRIANGLE_STRIP, 0, 4));
+
+    EXPECT_CALL(mock_gl, glDisableVertexAttribArray(texcoord_attr_loc));
+    EXPECT_CALL(mock_gl, glDisableVertexAttribArray(position_attr_loc));
+    EXPECT_CALL(mock_context_s, swap_buffers());
+    EXPECT_CALL(mock_gl, glUseProgram(0));
+
+    glprogram.render(renderlist, mock_context_s);
 }

=== modified file 'tests/unit-tests/graphics/android/test_hwc_fb_device.cpp'
--- tests/unit-tests/graphics/android/test_hwc_fb_device.cpp	2014-05-21 17:18:51 +0000
+++ tests/unit-tests/graphics/android/test_hwc_fb_device.cpp	2014-05-22 16:51:59 +0000
@@ -30,7 +30,7 @@
 #include "mir_test_doubles/mock_hwc_device_wrapper.h"
 #include "mir_test_doubles/stub_renderable_list_compositor.h"
 #include "mir_test_doubles/mock_renderable_list_compositor.h"
-#include "src/platform/graphics/android/overlay_gl_compositor.h"
+#include "src/platform/graphics/android/hwc_fallback_gl_renderer.h"
 #include "hwc_struct_helpers.h"
 #include <gtest/gtest.h>
 #include <stdexcept>

=== modified file 'tests/unit-tests/graphics/android/test_output_builder.cpp'
--- tests/unit-tests/graphics/android/test_output_builder.cpp	2014-05-07 02:56:33 +0000
+++ tests/unit-tests/graphics/android/test_output_builder.cpp	2014-05-22 16:51:59 +0000
@@ -28,6 +28,7 @@
 #include "mir_test_doubles/mock_android_hw.h"
 #include "mir_test_doubles/mock_fb_hal_device.h"
 #include "mir_test_doubles/mock_egl.h"
+#include "mir_test_doubles/mock_gl.h"
 #include "mir_test_doubles/mock_android_native_buffer.h"
 #include "mir_test_doubles/stub_gl_config.h"
 #include "mir_test_doubles/stub_gl_program_factory.h"
@@ -98,6 +99,7 @@
     }
 
     testing::NiceMock<mtd::MockEGL> mock_egl;
+    testing::NiceMock<mtd::MockGL> mock_gl;
     testing::NiceMock<mtd::HardwareAccessMock> hw_access_mock;
     testing::NiceMock<mtd::MockFBHalDevice> fb_hal_mock;
     std::shared_ptr<MockResourceFactory> mock_resource_factory;