Mir

Code review comment for lp:~kdub/mir/fix-1578159

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Sounds like the right fix. If we were preferring the latest buffer returned then we were accidentally double buffering instead of triple. Which would quantize to half frame rate more easily.

What concerns me though is that you've broken one aspect of the old test cases. In that you no longer expect minimal buffer allocation [1]:

88 - EXPECT_THAT(unique_ids_in(log), Eq(2));
89 + if (std::get<1>(GetParam()) == TestType::SubmitSemantics)
90 + EXPECT_THAT(log, NeverBlocks());
91 + if (std::get<1>(GetParam()) == TestType::ExchangeSemantics)
92 + EXPECT_THAT(unique_ids_in(log), Eq(2));

[1] https://docs.google.com/spreadsheets/d/1qf3IssN_sujygMPK2sTX2AUhGiDUBF3TMYYcSRSaSy0/pubhtml

That's a slight regression compared to BufferQueue. Although BufferQueue did not do it well to start with.

review: Abstain

« Back to merge proposal