ic
/
magnum
mirror of https://github.com/mosra/magnum.git
6
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

DebugTools: empty FrameProfiler shouldn't be implicitly enabled. 

That just makes no sense, as one then has to explicitly call disable()
to not have printStatistics() produce a useless line in the output.

Also have the same behavior also if it's constructed or set up from an
empty list of measurements, since that should be equivalent.
pull/680/head
Vladimír Vondruš 10 months ago
parent
73ec3a1444
commit
6e96f0e9dc
4 changed files with 87 additions and 16 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 16
      src/Magnum/DebugTools/FrameProfiler.cpp
  2. 42
      src/Magnum/DebugTools/FrameProfiler.h
  3. 1
      src/Magnum/DebugTools/Test/FrameProfilerGLTest.cpp
  4. 44
      src/Magnum/DebugTools/Test/FrameProfilerTest.cpp

16
src/Magnum/DebugTools/FrameProfiler.cpp
Unescape View File

@ -122,17 +122,18 @@ void FrameProfiler::setup(Containers::Array<Measurement>&& measurements, const U
}
#endif
/* Reset to have a clean slate in case we did some other measurements
before */
enable();
/* Reset to a clean slate in case we did some other measurements before,
then either disable or enable based on whether we have anything to
measure */
reset();
_measurements.isEmpty() ? disable() : enable();
}
void FrameProfiler::setup(const std::initializer_list<Measurement> measurements, const UnsignedInt maxFrameCount) {
setup(Containers::array(measurements), maxFrameCount);
}
void FrameProfiler::enable() {
_enabled = true;
void FrameProfiler::reset() {
#ifndef CORRADE_NO_ASSERT
_beginFrameCalled = false;
#endif
@ -148,6 +149,11 @@ void FrameProfiler::enable() {
}
}
void FrameProfiler::enable() {
reset();
_enabled = true;
}
void FrameProfiler::disable() {
_enabled = false;
}

42
src/Magnum/DebugTools/FrameProfiler.h
Unescape View File

@ -176,7 +176,9 @@ class MAGNUM_DEBUGTOOLS_EXPORT FrameProfiler {
/**
* @brief Default constructor
*
* Call @ref setup() to populate the profiler with measurements.
* Call @ref setup() to populate the profiler with measurements. A
* default-constructed instance has profiling implicitly disabled.
* @see @ref isEnabled()
*/
explicit FrameProfiler() noexcept;
@ -184,7 +186,9 @@ class MAGNUM_DEBUGTOOLS_EXPORT FrameProfiler {
* @brief Constructor
*
* Equivalent to default-constructing an instance and calling
* @ref setup() afterwards.
* @ref setup() afterwards. If @p measurements are not empty, profiling
* is implicitly enabled after, otherwise it's implicitly disabled.
* @see @ref isEnabled()
*/
explicit FrameProfiler(Containers::Array<Measurement>&& measurements, UnsignedInt maxFrameCount) noexcept;
@ -211,23 +215,38 @@ class MAGNUM_DEBUGTOOLS_EXPORT FrameProfiler {
*
* Calling @ref setup() on an already set up profiler will replace
* existing measurements with @p measurements and reset
* @ref measuredFrameCount() back to @cpp 0 @ce.
* @ref measuredFrameCount() back to @cpp 0 @ce. If @p measurements
* are not empty, profiling is implicitly enabled after, otherwise it's
* implicitly disabled.
* @see @ref isEnabled()
*/
void setup(Containers::Array<Measurement>&& measurements, UnsignedInt maxFrameCount);
/** @overload */
void setup(std::initializer_list<Measurement> measurements, UnsignedInt maxFrameCount);
/** @brief Whether the profiling is enabled */
/**
* @brief Whether the profiling is enabled
*
* @see @ref enable(), @ref disable(), @ref setup()
*/
bool isEnabled() const { return _enabled; }
/**
* @brief Enable the profiler
*
* The profiler is enabled implicitly after construction. When this
* function is called, it discards all measured data, effectively
* making @ref measuredFrameCount() zero. If you want to reset the
* profiler to measure different values as well, call @ref setup().
* When this function is called, it discards all measured data,
* effectively making @ref measuredFrameCount() zero. If you want to
* reset the profiler to measure different values as well, call
* @ref setup() instead.
*
* A default-constructed profiler is implicitly disabled as it has
* nothing to measure. Constructing it with a non-empty set of
* measurements or calling @ref setup() with a non-empty set of
* measurements makes it implicitly enabled. It's allowed to call this
* function even if the profiler has no measurements, although it'll
* result in @ref statistics() / @ref printStatistics() producing
* useless output.
*/
void enable();
@ -381,8 +400,8 @@ class MAGNUM_DEBUGTOOLS_EXPORT FrameProfiler {
* but in addition, if the output is a TTY, it's colored and overwrites
* itself instead of filling up the terminal history. If profiling is
* disabled, does nothing.
* @see @ref isMeasurementAvailable(), @ref isEnabled()
* @ref Corrade::Utility::Debug::isTty()
* @see @ref isMeasurementAvailable(), @ref isEnabled(),
* @relativeref{Corrade,Utility::Debug::isTty()}
*/
void printStatistics(UnsignedInt frequency) const;
@ -404,11 +423,12 @@ class MAGNUM_DEBUGTOOLS_EXPORT FrameProfiler {
}
private:
void reset();
UnsignedInt delayedCurrentData(UnsignedInt delay) const;
Double measurementMeanInternal(const Measurement& measurement) const;
void printStatisticsInternal(Debug& out) const;
bool _enabled = true;
bool _enabled = false;
#ifndef CORRADE_NO_ASSERT
/* Here it shouldn't cause the class to have a different size when
asserts get disabled */

1
src/Magnum/DebugTools/Test/FrameProfilerGLTest.cpp
Unescape View File

@ -122,6 +122,7 @@ void FrameProfilerGLTest::test() {
Shaders::FlatGL3D shader;
FrameProfilerGL profiler{data.values, 4};
CORRADE_VERIFY(profiler.isEnabled());
CORRADE_COMPARE(profiler.maxFrameCount(), 4);
/* MSVC 2015 needs the {} */

44
src/Magnum/DebugTools/Test/FrameProfilerTest.cpp
Unescape View File

@ -158,11 +158,20 @@ FrameProfilerTest::FrameProfilerTest() {
void FrameProfilerTest::defaultConstructed() {
FrameProfiler profiler;
CORRADE_VERIFY(!profiler.isEnabled());
CORRADE_COMPARE(profiler.maxFrameCount(), 1);
CORRADE_COMPARE(profiler.measuredFrameCount(), 0);
CORRADE_COMPARE(profiler.measurementCount(), 0);
CORRADE_COMPARE(profiler.statistics(), "Last 0 frames:");
/* Nothing happens by default as it's not enabled */
profiler.beginFrame();
profiler.endFrame();
CORRADE_COMPARE(profiler.measuredFrameCount(), 0);
profiler.enable();
CORRADE_VERIFY(profiler.isEnabled());
profiler.beginFrame();
profiler.endFrame();
CORRADE_COMPARE(profiler.measuredFrameCount(), 1);
@ -176,11 +185,20 @@ void FrameProfilerTest::defaultConstructed() {
void FrameProfilerTest::noMeasurements() {
FrameProfiler profiler{{}, 3};
CORRADE_VERIFY(!profiler.isEnabled());
CORRADE_COMPARE(profiler.maxFrameCount(), 3);
CORRADE_COMPARE(profiler.measuredFrameCount(), 0);
CORRADE_COMPARE(profiler.measurementCount(), 0);
CORRADE_COMPARE(profiler.statistics(), "Last 0 frames:");
/* Nothing happens by default as it's not enabled */
profiler.beginFrame();
profiler.endFrame();
CORRADE_COMPARE(profiler.measuredFrameCount(), 0);
profiler.enable();
CORRADE_VERIFY(profiler.isEnabled());
profiler.beginFrame();
profiler.endFrame();
CORRADE_COMPARE(profiler.measuredFrameCount(), 1);
@ -213,6 +231,8 @@ void FrameProfilerTest::singleFrame() {
UnsignedLong time = 0, memory = 50;
FrameProfiler profiler;
CORRADE_VERIFY(!profiler.isEnabled());
if(!data.delayed) {
profiler.setup({
FrameProfiler::Measurement{
@ -279,6 +299,7 @@ void FrameProfilerTest::singleFrame() {
}, nullptr}
}, 1);
}
CORRADE_VERIFY(profiler.isEnabled());
CORRADE_COMPARE(profiler.maxFrameCount(), 1);
CORRADE_COMPARE(profiler.measuredFrameCount(), 0);
CORRADE_COMPARE(profiler.measurementCount(), 3);
@ -367,6 +388,8 @@ void FrameProfilerTest::multipleFrames() {
UnsignedInt delay;
} state {0, 50, {0, 0, 0}, {50, 0, 0}, data.delay};
FrameProfiler profiler;
CORRADE_VERIFY(!profiler.isEnabled());
if(!data.delayed) {
profiler.setup({
FrameProfiler::Measurement{
@ -441,6 +464,7 @@ void FrameProfilerTest::multipleFrames() {
}, nullptr}
}, 3);
}
CORRADE_VERIFY(profiler.isEnabled());
CORRADE_COMPARE(profiler.maxFrameCount(), 3);
CORRADE_COMPARE(profiler.measuredFrameCount(), 0);
CORRADE_COMPARE(profiler.measurementDelay(0), data.delay);
@ -580,6 +604,7 @@ void FrameProfilerTest::enableDisable() {
profiler.endFrame();
profiler.beginFrame();
profiler.endFrame();
CORRADE_VERIFY(profiler.isEnabled());
CORRADE_COMPARE(profiler.measurementCount(), 1);
CORRADE_COMPARE(profiler.measuredFrameCount(), 3);
CORRADE_COMPARE(profiler.measurementDelay(0), 2);
@ -588,6 +613,7 @@ void FrameProfilerTest::enableDisable() {
/* It should only freeze everything, not wipe out any data */
profiler.disable();
CORRADE_VERIFY(!profiler.isEnabled());
CORRADE_COMPARE(profiler.measurementCount(), 1);
CORRADE_COMPARE(profiler.measuredFrameCount(), 3);
CORRADE_COMPARE(profiler.measurementDelay(0), 2);
@ -607,6 +633,7 @@ void FrameProfilerTest::enableDisable() {
/* Enabling should reset the data to have a clean slate, but not the
measurements */
profiler.enable();
CORRADE_VERIFY(profiler.isEnabled());
CORRADE_COMPARE(profiler.measurementCount(), 1);
CORRADE_COMPARE(profiler.maxFrameCount(), 5);
CORRADE_COMPARE(profiler.measuredFrameCount(), 0);
@ -638,10 +665,13 @@ void FrameProfilerTest::reSetup() {
[](void*, UnsignedInt) {},
[](void*, UnsignedInt, UnsignedInt) { return UnsignedLong{}; }, nullptr},
}, 5};
CORRADE_VERIFY(profiler.isEnabled());
profiler.beginFrame();
profiler.endFrame();
profiler.beginFrame();
CORRADE_COMPARE(profiler.measurementCount(), 1);
CORRADE_COMPARE(profiler.measuredFrameCount(), 1);
/* Setup should replace everything */
profiler.setup({
@ -656,6 +686,7 @@ void FrameProfilerTest::reSetup() {
[](void*) { return UnsignedLong{}; },
nullptr},
}, 10);
CORRADE_VERIFY(profiler.isEnabled());
CORRADE_COMPARE(profiler.measurementCount(), 2);
CORRADE_COMPARE(profiler.maxFrameCount(), 10);
CORRADE_COMPARE(profiler.measuredFrameCount(), 0);
@ -668,6 +699,14 @@ void FrameProfilerTest::reSetup() {
beginFrame() expected again */
profiler.beginFrame();
profiler.endFrame();
CORRADE_COMPARE(profiler.measuredFrameCount(), 1);
/* Empty setup clears everything and makes it disabled */
profiler.setup({}, 17);
CORRADE_VERIFY(!profiler.isEnabled());
CORRADE_COMPARE(profiler.measurementCount(), 0);
CORRADE_COMPARE(profiler.maxFrameCount(), 17);
CORRADE_COMPARE(profiler.measuredFrameCount(), 0);
}
void FrameProfilerTest::copy() {
@ -811,7 +850,11 @@ void FrameProfilerTest::delayTooLittleFrames() {
void FrameProfilerTest::startStopFrameUnexpected() {
CORRADE_SKIP_IF_NO_ASSERT();
/* Empty profiler is disabled by default, enable it to not have
beginFrame() and endFrame() no-op */
FrameProfiler profiler;
profiler.enable();
CORRADE_VERIFY(profiler.isEnabled());
Containers::String out;
{
@ -1101,6 +1144,7 @@ void FrameProfilerTest::gl() {
Containers::Pointer<FrameProfilerGL> profiler_{InPlaceInit, data.values, 4u};
FrameProfilerGL profiler = Utility::move(*profiler_);
profiler_ = nullptr;
CORRADE_COMPARE(profiler.isEnabled(), !!data.values);
CORRADE_COMPARE(profiler.values(), data.values);
CORRADE_COMPARE(profiler.maxFrameCount(), 4);
CORRADE_COMPARE(profiler.measurementCount(), data.measurementCount);

Write Preview
Loading…
Cancel
Save