Mir

Merge lp:~kdub/mir/fix-1352883 into lp:mir

  1. fix-1352883
  2. Merge into development-branch
Proposed by Kevin DuBois
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 1837
Proposed branch: lp:~kdub/mir/fix-1352883
Merge into: lp:mir
Diff against target: 256 lines (+210/-3)
3 files modified
src/platform/graphics/android/hwc_device.cpp (+19/-3)
src/platform/graphics/android/hwc_device.h (+1/-0)
tests/unit-tests/graphics/android/test_hwc_device.cpp (+190/-0)
To merge this branch: bzr merge lp:~kdub/mir/fix-1352883
Reviewer Review Type Date Requested Status
Alan Griffiths Approve
Daniel van Vugt Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+229804@code.launchpad.net

Commit message

android: fix intermittent problem that happens during certain list rearrangements. Scan the list of onscreen buffers in HwcDevice instead of trying to have the list helper classes figure out the onscreen buffers.
(LP: #1352883)

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1827
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2332/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1229
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1235
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1218
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/854
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/854/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/855
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/855/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/153
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/153/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2334
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11093

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

review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Tested on N4, seems to work. I'm not familiar with the affected Android code though.

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

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/platform/graphics/android/hwc_device.cpp'
--- src/platform/graphics/android/hwc_device.cpp 2014-08-01 09:14:20 +0000
+++ src/platform/graphics/android/hwc_device.cpp 2014-08-06 14:48:05 +0000
@@ -26,6 +26,7 @@
26#include "buffer.h"26#include "buffer.h"
27#include "hwc_fallback_gl_renderer.h"27#include "hwc_fallback_gl_renderer.h"
28#include <limits>28#include <limits>
29#include <algorithm>
2930
30namespace mg = mir::graphics;31namespace mg = mir::graphics;
31namespace mga=mir::graphics::android;32namespace mga=mir::graphics::android;
@@ -74,6 +75,19 @@
74{75{
75}76}
7677
78bool mga::HwcDevice::buffer_is_onscreen(mg::Buffer const& buffer) const
79{
80 /* check the handles, as the buffer ptrs might change between sets */
81 auto const handle = buffer.native_buffer_handle().get();
82 auto it = std::find_if(
83 onscreen_overlay_buffers.begin(), onscreen_overlay_buffers.end(),
84 [&handle](std::shared_ptr<mg::Buffer> const& b)
85 {
86 return (handle == b->native_buffer_handle().get());
87 });
88 return it != onscreen_overlay_buffers.end();
89}
90
77void mga::HwcDevice::post_gl(SwappingGLContext const& context)91void mga::HwcDevice::post_gl(SwappingGLContext const& context)
78{92{
79 auto lg = lock_unblanked();93 auto lg = lock_unblanked();
@@ -142,9 +156,11 @@
142 }156 }
143 else157 else
144 {158 {
145 if (it->needs_commit)159 auto buffer = renderable->buffer();
146 it->layer.set_acquirefence_from(*renderable->buffer());160 if (!buffer_is_onscreen(*buffer))
147 next_onscreen_overlay_buffers.push_back(renderable->buffer());161 it->layer.set_acquirefence_from(*buffer);
162
163 next_onscreen_overlay_buffers.push_back(buffer);
148 }164 }
149 it++;165 it++;
150 }166 }
151167
=== modified file 'src/platform/graphics/android/hwc_device.h'
--- src/platform/graphics/android/hwc_device.h 2014-08-01 09:14:20 +0000
+++ src/platform/graphics/android/hwc_device.h 2014-08-06 14:48:05 +0000
@@ -52,6 +52,7 @@
52 RenderableListCompositor const& list_compositor);52 RenderableListCompositor const& list_compositor);
5353
54private:54private:
55 bool buffer_is_onscreen(Buffer const&) const;
55 void turned_screen_off() override;56 void turned_screen_off() override;
56 LayerList hwc_list;57 LayerList hwc_list;
57 std::vector<std::shared_ptr<Buffer>> onscreen_overlay_buffers;58 std::vector<std::shared_ptr<Buffer>> onscreen_overlay_buffers;
5859
=== modified file 'tests/unit-tests/graphics/android/test_hwc_device.cpp'
--- tests/unit-tests/graphics/android/test_hwc_device.cpp 2014-08-01 09:14:20 +0000
+++ tests/unit-tests/graphics/android/test_hwc_device.cpp 2014-08-06 14:48:05 +0000
@@ -583,3 +583,193 @@
583 device.mode(MirPowerMode::mir_power_mode_off);583 device.mode(MirPowerMode::mir_power_mode_off);
584 EXPECT_THAT(stub_buffer1.use_count(), Eq(use_count_before));584 EXPECT_THAT(stub_buffer1.use_count(), Eq(use_count_before));
585}585}
586
587TEST_F(HwcDevice, tracks_hwc_owned_fences_even_across_list_changes)
588{
589 using namespace testing;
590 hwc_layer_1_t layer3;
591 fill_hwc_layer(layer2, &comp_rect, position1, *stub_buffer1, HWC_FRAMEBUFFER, 0);
592 fill_hwc_layer(layer3, &comp2_rect, position2, *stub_buffer2, HWC_FRAMEBUFFER, 0);
593
594 int acquire_fence1 = 39303;
595 int acquire_fence2 = 393044;
596 int release_fence1 = 39304;
597 int release_fence2 = 123;
598 int release_fence3 = 136;
599 mg::RenderableList renderlist1{
600 stub_renderable1
601 };
602 mg::RenderableList renderlist2{
603 stub_renderable1,
604 stub_renderable2
605 };
606
607 layer.acquireFenceFd = acquire_fence1;
608 layer.compositionType = HWC_OVERLAY;
609 std::list<hwc_layer_1_t*> expected_list1
610 {
611 &layer,
612 &target_layer
613 };
614 auto set_fences = [&](hwc_display_contents_1_t& contents)
615 {
616 ASSERT_EQ(contents.numHwLayers, 2);
617 contents.hwLayers[0].releaseFenceFd = release_fence1;
618 contents.retireFenceFd = -1;
619 };
620 auto set_fences2 = [&](hwc_display_contents_1_t& contents)
621 {
622 ASSERT_EQ(contents.numHwLayers, 3);
623 contents.hwLayers[0].releaseFenceFd = release_fence2;
624 contents.hwLayers[1].releaseFenceFd = release_fence3;
625 contents.retireFenceFd = -1;
626 };
627
628 layer2.acquireFenceFd = -1;
629 layer2.compositionType = HWC_OVERLAY;
630 layer3.acquireFenceFd = acquire_fence2;
631 layer3.compositionType = HWC_OVERLAY;
632 std::list<hwc_layer_1_t*> expected_list2
633 {
634 &layer2,
635 &layer3,
636 &target_layer
637 };
638
639 Sequence seq;
640 // first post
641 EXPECT_CALL(*mock_device, prepare(_))
642 .InSequence(seq)
643 .WillOnce(Invoke(set_all_layers_to_overlay));
644 EXPECT_CALL(*mock_native_buffer1, copy_fence())
645 .InSequence(seq)
646 .WillOnce(Return(acquire_fence1));
647 EXPECT_CALL(*mock_device, set(MatchesList(expected_list1)))
648 .InSequence(seq)
649 .WillOnce(Invoke(set_fences));
650 EXPECT_CALL(*mock_native_buffer1, update_usage(release_fence1, mga::BufferAccess::read))
651 .InSequence(seq);
652
653 //end first post, second post
654 EXPECT_CALL(*mock_device, prepare(_))
655 .InSequence(seq)
656 .WillOnce(Invoke(set_all_layers_to_overlay));
657 //note that only the 2nd buffer should have its fence copied
658 EXPECT_CALL(*mock_native_buffer2, copy_fence())
659 .InSequence(seq)
660 .WillOnce(Return(acquire_fence2));
661 EXPECT_CALL(*mock_device, set(MatchesList(expected_list2)))
662 .InSequence(seq)
663 .WillOnce(Invoke(set_fences2));
664 EXPECT_CALL(*mock_native_buffer1, update_usage(release_fence2, mga::BufferAccess::read))
665 .InSequence(seq);
666 EXPECT_CALL(*mock_native_buffer2, update_usage(release_fence3, mga::BufferAccess::read))
667 .InSequence(seq);
668 //end second post
669
670 mga::HwcDevice device(mock_device, mock_vsync, mock_file_ops);
671 EXPECT_TRUE(device.post_overlays(stub_context, renderlist1, stub_compositor));
672
673 EXPECT_TRUE(device.post_overlays(stub_context, renderlist2, stub_compositor));
674}
675
676TEST_F(HwcDevice, tracks_hwc_owned_fences_across_list_rearrange)
677{
678 using namespace testing;
679 hwc_layer_1_t layer3;
680 hwc_layer_1_t layer4;
681 fill_hwc_layer(layer3, &comp2_rect, position2, *stub_buffer2, HWC_FRAMEBUFFER, 0);
682 fill_hwc_layer(layer4, &comp_rect, position1, *stub_buffer1, HWC_FRAMEBUFFER, 0);
683
684 int acquire_fence1 = 39303;
685 int acquire_fence2 = 393044;
686 int release_fence1 = 39304;
687 int release_fence2 = 123;
688 int release_fence3 = 136;
689 int release_fence4 = 1344;
690 mg::RenderableList renderlist{
691 stub_renderable1,
692 stub_renderable2
693 };
694
695 //the renderable ptr or the buffer ptr could change, while still referencing the same buffer_handle_t
696 mg::RenderableList renderlist2{
697 std::make_shared<mtd::StubRenderable>(
698 std::make_shared<mtd::StubBuffer>(mock_native_buffer2, size2), position2),
699 std::make_shared<mtd::StubRenderable>(
700 std::make_shared<mtd::StubBuffer>(mock_native_buffer1, size1), position1),
701 };
702
703 layer.acquireFenceFd = acquire_fence1;
704 layer.compositionType = HWC_OVERLAY;
705 layer2.acquireFenceFd = acquire_fence2;
706 layer2.compositionType = HWC_OVERLAY;
707 std::list<hwc_layer_1_t*> expected_list1
708 {
709 &layer,
710 &layer2,
711 &target_layer
712 };
713 auto set_fences = [&](hwc_display_contents_1_t& contents)
714 {
715 ASSERT_EQ(contents.numHwLayers, 3);
716 contents.hwLayers[0].releaseFenceFd = release_fence1;
717 contents.hwLayers[1].releaseFenceFd = release_fence2;
718 contents.retireFenceFd = -1;
719 };
720 auto set_fences2 = [&](hwc_display_contents_1_t& contents)
721 {
722 ASSERT_EQ(contents.numHwLayers, 3);
723 contents.hwLayers[0].releaseFenceFd = release_fence3;
724 contents.hwLayers[1].releaseFenceFd = release_fence4;
725 contents.retireFenceFd = -1;
726 };
727
728 layer3.acquireFenceFd = -1;
729 layer3.compositionType = HWC_OVERLAY;
730 layer4.acquireFenceFd = -1;
731 layer4.compositionType = HWC_OVERLAY;
732 std::list<hwc_layer_1_t*> expected_list2
733 {
734 &layer3,
735 &layer4,
736 &target_layer
737 };
738
739 Sequence seq;
740 // first post
741 EXPECT_CALL(*mock_device, prepare(_))
742 .InSequence(seq)
743 .WillOnce(Invoke(set_all_layers_to_overlay));
744 EXPECT_CALL(*mock_native_buffer1, copy_fence())
745 .InSequence(seq)
746 .WillOnce(Return(acquire_fence1));
747 EXPECT_CALL(*mock_native_buffer2, copy_fence())
748 .InSequence(seq)
749 .WillOnce(Return(acquire_fence2));
750 EXPECT_CALL(*mock_device, set(MatchesList(expected_list1)))
751 .InSequence(seq)
752 .WillOnce(Invoke(set_fences));
753 EXPECT_CALL(*mock_native_buffer1, update_usage(release_fence1, mga::BufferAccess::read))
754 .InSequence(seq);
755 EXPECT_CALL(*mock_native_buffer2, update_usage(release_fence2, mga::BufferAccess::read))
756 .InSequence(seq);
757
758 //end first post, second post
759 EXPECT_CALL(*mock_device, prepare(_))
760 .InSequence(seq)
761 .WillOnce(Invoke(set_all_layers_to_overlay));
762 //note that the buffers just flipped position, no acquire fence copying.
763 EXPECT_CALL(*mock_device, set(MatchesList(expected_list2)))
764 .InSequence(seq)
765 .WillOnce(Invoke(set_fences2));
766 EXPECT_CALL(*mock_native_buffer2, update_usage(release_fence3, mga::BufferAccess::read))
767 .InSequence(seq);
768 EXPECT_CALL(*mock_native_buffer1, update_usage(release_fence4, mga::BufferAccess::read))
769 .InSequence(seq);
770 //end second post
771
772 mga::HwcDevice device(mock_device, mock_vsync, mock_file_ops);
773 EXPECT_TRUE(device.post_overlays(stub_context, renderlist, stub_compositor));
774 EXPECT_TRUE(device.post_overlays(stub_context, renderlist2, stub_compositor));
775}

Subscribers

People subscribed via source and target branches