Mir

Merge lp://qastaging/~afrantzis/mir/texture-bindable into lp://qastaging/mir

Proposed by Alexandros Frantzis
Status: Rejected
Rejected by: Alexandros Frantzis
Proposed branch: lp://qastaging/~afrantzis/mir/texture-bindable
Merge into: lp://qastaging/mir
Prerequisite: lp://qastaging/~afrantzis/mir/buffer-native-type
Diff against target: 946 lines (+316/-55)
32 files modified
examples/CMakeLists.txt (+2/-0)
examples/buffer_render_target.cpp (+7/-1)
examples/server_example_adorning_compositor.cpp (+12/-1)
include/platform/mir/graphics/buffer.h (+0/-1)
src/platforms/CMakeLists.txt (+1/-0)
src/platforms/android/server/buffer.h (+3/-1)
src/platforms/mesa/server/common/gbm_buffer.h (+3/-1)
src/platforms/mesa/server/common/shm_buffer.h (+3/-1)
src/renderers/gl/include/mir/renderer/gl/texture_bindable.h (+46/-0)
src/server/compositor/CMakeLists.txt (+4/-0)
src/server/compositor/recently_used_cache.cpp (+11/-1)
src/server/compositor/screencast_display_buffer.cpp (+8/-2)
src/server/compositor/screencast_display_buffer.h (+2/-0)
src/server/compositor/temporary_buffers.cpp (+0/-5)
src/server/compositor/temporary_buffers.h (+0/-1)
src/server/scene/CMakeLists.txt (+4/-0)
src/server/scene/gl_pixel_buffer.cpp (+7/-1)
tests/CMakeLists.txt (+1/-0)
tests/include/mir/test/doubles/mock_buffer.h (+4/-1)
tests/include/mir/test/doubles/mock_gl_buffer.h (+45/-0)
tests/include/mir/test/doubles/stub_buffer.h (+0/-1)
tests/include/mir/test/doubles/stub_gl_buffer.h (+45/-0)
tests/include/mir/test/doubles/stub_gl_buffer_allocator.h (+45/-0)
tests/integration-tests/graphics/mesa/test_buffer_integration.cpp (+12/-3)
tests/integration-tests/test_session.cpp (+21/-1)
tests/unit-tests/compositor/test_compositing_screencast.cpp (+6/-6)
tests/unit-tests/compositor/test_gl_renderer.cpp (+3/-3)
tests/unit-tests/compositor/test_gl_texture_cache.cpp (+4/-3)
tests/unit-tests/compositor/test_screencast_display_buffer.cpp (+8/-8)
tests/unit-tests/compositor/test_temporary_buffers.cpp (+0/-9)
tests/unit-tests/graphics/mesa/kms/test_gbm_buffer.cpp (+7/-2)
tests/unit-tests/scene/test_gl_pixel_buffer.cpp (+2/-2)
To merge this branch: bzr merge lp://qastaging/~afrantzis/mir/texture-bindable
Reviewer Review Type Date Requested Status
Alan Griffiths Needs Fixing
Cemil Azizoglu (community) Needs Information
Chris Halse Rogers Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+269231@code.qastaging.launchpad.net

Commit message

platform: Move gl_bind_to_texture method from Buffer to mir::renderer::gl::TextureBindable

Description of the change

platform: Move gl_bind_to_texture method from Buffer to mir::renderer::gl::TextureBindable

This MP makes the Buffer class agnostic of rendering technology, by moving the gl_bind_to_buffer() method to a renderer specific interface (TextureBindable). This interface is implemented by the native buffer types and accessed by components that need it by dynamically casting Buffer::native_buffer() to it. This will hopefully give an idea to the team about how other renderer types are expected to function.

The definition of TextureBindable will eventually be made public (e.g. shipped in a mir-renderer-gl-dev package), so I have made it accessible from most of the non-core codebase (tests and examples). The core (libmirserver), however, should not be aware of the rendering-specific technologies, so I've tried to limit exposure to TextureBindable only to parts that currently need it (e.g. scene, compositor). These components will be eventually cleaned to remove this dependency.

Next steps involve isolating the GL dependencies from our core, so they can be moved into a different shared object (i.e. plugin).

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

Looks sensible.

Non-blocking: It might be nice to provide TextureBindable* as_texture_bindable(mg::Buffer*) as a helper method; the logic seems duplicated enough to warrant it.

It would also make it easier to play with different ways of extracting the TextureBinadble (dynamic_cast, our own hand-rolled RTTI, whatever). Should we find that we want to do so.

review: Approve
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

1) src/renderers/gl/include/mir/renderer/gl/texture_bindable.h
Do we need both 'gl's in the path?

2) Have you built and tested with mesa-x11 as the first platform on the list? (May need lp:~cemil-azizoglu/mir/fix-tests-when-only-x11-defined)

review: Needs Information
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

16 + ${PROJECT_SOURCE_DIR}/src/renderers/gl/include

"The definition of TextureBindable will eventually be made public (e.g. shipped in a mir-renderer-gl-dev package), so I have made it accessible from most of the non-core codebase (tests and examples)."

I'd be happier with putting the header under include and making it public than having this "temporary" solution.

If we do need a temporary location for pre-public files I'd prefer a name like "${PROJECT_SOURCE_DIR}/src/pre-public-include" to burying it whatever part of the source tree is publishing that specific file.

review: Needs Fixing
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Superseded by https://code.launchpad.net/~afrantzis/mir/texture-bindable-over-native-buffer-base/+merge/269739, which is the same branch using Buffer::native_buffer_base() (which is the approach we chose) instead of the original Buffer::native_type().

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

I will reply to the existing comments in the new branch.

Unmerged revisions

2881. By Alexandros Frantzis

platform: Move gl_bind_to_texture method from Buffer to mir::renderer::gl::TextureBindable

2880. By Alexandros Frantzis

platform: Introduce Buffer::native_type() method

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches