do not prefer framebuffer configs with excessive multisamples (#1456)

Co-authored-by: user1 <[email protected]>

panda/src/display/config_display.cxx

@@ -462,7 +462,8 @@ ConfigVariableInt accum_bits
  PRC_DESC("The minimum number of accumulator buffer bits requested."));
 ConfigVariableInt multisamples
 ("multisamples", 0,
- PRC_DESC("The minimum number of samples requested."));
+ PRC_DESC("The number of samples requested. Set this to 1 to request "
+          "the maximum number of samples available"));
 ConfigVariableInt back_buffers
 ("back-buffers", 1,
  PRC_DESC("The default number of back buffers requested."));

panda/src/display/frameBufferProperties.cxx

@@ -525,13 +525,38 @@ get_quality(const FrameBufferProperties &reqs) const {
     }
   }
 
-  // deduct for insufficient multisamples.  Cost: 1,000
+  // Deduct for insufficient multisamples.  Cost: 1,000
 
   if (_property[FBP_multisamples] != 0 &&
       reqs._property[FBP_multisamples] > _property[FBP_multisamples]) {
     quality -= 1000;
   }
 
+  // Deduct for having more multisamples than requested.  Cost: 2 per sample
+  // In case the special value of 1 multisample is requested, nothing gets deducted.
+
+  if (_property[FBP_multisamples] != 0 &&
+      reqs._property[FBP_multisamples] != 1 &&
+      reqs._property[FBP_multisamples] < _property[FBP_multisamples]) {
+    quality -= 2 * _property[FBP_multisamples];
+  }
+
+  // Deduct for insufficient coverage samples.  Cost: 1,000
+
+  if (_property[FBP_coverage_samples] != 0 &&
+    reqs._property[FBP_coverage_samples] > _property[FBP_coverage_samples]) {
+    quality -= 1000;
+  }
+
+  // Deduct for having more coverage samples than requested.  Cost: 2 per sample
+  // In case the special value of 1 sample is requested, nothing gets deducted.
+
+  if (_property[FBP_coverage_samples] != 0 &&
+    reqs._property[FBP_coverage_samples] != 1 &&
+    reqs._property[FBP_coverage_samples] < _property[FBP_coverage_samples]) {
+    quality -= 2 * _property[FBP_coverage_samples];
+  }
+
   // Deduct for unrequested bitplanes.  Cost: 50
 
   for (int prop = FBP_depth_bits; prop <= FBP_accum_bits; ++prop) {
@@ -566,6 +591,13 @@ get_quality(const FrameBufferProperties &reqs) const {
     quality -= 100;
   }
 
+  // Bonus for each multisample in case the special value of 1 multisample
+  // is requested.  Extra: 2 per sample.
+  if (_property[FBP_multisamples] != 0 &&
+      reqs._property[FBP_multisamples] == 1) {
+    quality += 2 * _property[FBP_multisamples];
+  }
+
   // Bonus for each depth bit.  Extra: 8 per bit.
   // Please note that the Intel Windows driver only gives extra depth in
   // combination with a stencil buffer, so we need 8 extra depth bits to
@@ -575,13 +607,10 @@ get_quality(const FrameBufferProperties &reqs) const {
     quality += 8 * _property[FBP_depth_bits];
   }
 
-  // Bonus for each multisample.  Extra: 2 per sample.
-  if (reqs._property[FBP_multisamples] != 0) {
-    quality += 2 * _property[FBP_multisamples];
-  }
-
-  // Bonus for each coverage sample.  Extra: 2 per sample.
-  if (reqs._property[FBP_coverage_samples] != 0) {
+  // Bonus for each coverage sample in case the special value of 1 sample
+  // is requested.  Extra: 2 per sample.
+  if (_property[FBP_coverage_samples] != 0 &&
+    reqs._property[FBP_coverage_samples] == 1) {
     quality += 2 * _property[FBP_coverage_samples];
   }
 

tests/display/test_fbprops.py

@@ -101,3 +101,166 @@ def test_fbquality_rgba64():
     assert fb_rgba8.get_quality(req_rgb1) > fb_rgba16.get_quality(req_rgb1)
     assert fb_rgba8.get_quality(req_rgb0_alpha0) > fb_rgba16.get_quality(req_rgb0_alpha0)
     assert fb_rgba8.get_quality(req_rgb1_alpha1) > fb_rgba16.get_quality(req_rgb1_alpha1)
+
+
+def test_fbquality_multi_samples():
+    # Make sure that we get appropriate quality levels for different sample requests
+    
+    fb_0_samples = FrameBufferProperties()
+    fb_0_samples.set_rgb_color(1)
+    fb_0_samples.set_multisamples(0)
+    
+    fb_2_samples = FrameBufferProperties()
+    fb_2_samples.set_rgb_color(1)
+    fb_2_samples.set_multisamples(2)
+    
+    fb_4_samples = FrameBufferProperties()
+    fb_4_samples.set_rgb_color(1)
+    fb_4_samples.set_multisamples(4)
+    
+    fb_8_samples = FrameBufferProperties()
+    fb_8_samples.set_rgb_color(1)
+    fb_8_samples.set_multisamples(8)
+    
+    fb_16_samples = FrameBufferProperties()
+    fb_16_samples.set_rgb_color(1)
+    fb_16_samples.set_multisamples(16)
+    
+    req_0_samples = FrameBufferProperties()
+    req_0_samples.set_multisamples(0)
+    
+    req_1_samples = FrameBufferProperties()
+    req_1_samples.set_multisamples(1)
+    
+    req_2_samples = FrameBufferProperties()
+    req_2_samples.set_multisamples(2)
+    
+    req_4_samples = FrameBufferProperties()
+    req_4_samples.set_multisamples(4)
+    
+    req_8_samples = FrameBufferProperties()
+    req_8_samples.set_multisamples(8)
+    
+    req_16_samples = FrameBufferProperties()
+    req_16_samples.set_multisamples(16)
+
+    # a fb which does not provide the requested number of samples should always 
+    # have a lower quality than another
+    assert fb_2_samples.get_quality(req_4_samples) < fb_2_samples.get_quality(req_2_samples)
+    assert fb_2_samples.get_quality(req_4_samples) < fb_4_samples.get_quality(req_2_samples)
+    assert fb_2_samples.get_quality(req_4_samples) < fb_8_samples.get_quality(req_2_samples)
+    assert fb_2_samples.get_quality(req_4_samples) < fb_16_samples.get_quality(req_2_samples)
+    assert fb_2_samples.get_quality(req_4_samples) < fb_16_samples.get_quality(req_16_samples)
+    assert fb_8_samples.get_quality(req_16_samples) < fb_2_samples.get_quality(req_2_samples)
+    
+    # a fb which has more than the requested samples should have a 
+    # lower quality than one that matches exactly
+    assert fb_2_samples.get_quality(req_2_samples) > fb_4_samples.get_quality(req_2_samples)
+    assert fb_2_samples.get_quality(req_2_samples) > fb_8_samples.get_quality(req_2_samples)
+    assert fb_2_samples.get_quality(req_2_samples) > fb_16_samples.get_quality(req_2_samples)
+    assert fb_2_samples.get_quality(req_2_samples) > fb_16_samples.get_quality(req_8_samples)
+    
+    # the more additional samples the fb provides rather than what is requested, 
+    # the lower the quality should be
+    assert fb_16_samples.get_quality(req_2_samples) < fb_8_samples.get_quality(req_2_samples)
+    assert fb_8_samples.get_quality(req_2_samples) < fb_4_samples.get_quality(req_2_samples)
+    
+    # if the special value of 1 sample is requested, the fb with the highest samples should be 
+    # in favour
+    assert fb_16_samples.get_quality(req_1_samples) > fb_8_samples.get_quality(req_1_samples)
+    assert fb_16_samples.get_quality(req_1_samples) > fb_4_samples.get_quality(req_1_samples)
+    assert fb_16_samples.get_quality(req_1_samples) > fb_2_samples.get_quality(req_1_samples)
+    assert fb_8_samples.get_quality(req_1_samples) > fb_4_samples.get_quality(req_1_samples)
+    assert fb_8_samples.get_quality(req_1_samples) > fb_2_samples.get_quality(req_1_samples)
+    
+    # if 0 samples are requested, the fb with the highest samples should get a reduced quality level
+    assert fb_16_samples.get_quality(req_0_samples) < fb_8_samples.get_quality(req_0_samples)
+    assert fb_16_samples.get_quality(req_0_samples) < fb_4_samples.get_quality(req_0_samples)
+    assert fb_16_samples.get_quality(req_0_samples) < fb_2_samples.get_quality(req_0_samples)
+    assert fb_8_samples.get_quality(req_0_samples) < fb_4_samples.get_quality(req_0_samples)
+    assert fb_8_samples.get_quality(req_0_samples) < fb_2_samples.get_quality(req_0_samples)
+
+    # if samples are requested we prefer the ones with samples instead of having none
+    assert fb_0_samples.get_quality(req_2_samples) < fb_2_samples.get_quality(req_4_samples)
+    assert fb_0_samples.get_quality(req_2_samples) < fb_2_samples.get_quality(req_8_samples)
+    assert fb_0_samples.get_quality(req_2_samples) < fb_2_samples.get_quality(req_16_samples)
+    
+    # we prefer buffers without samples if we don't request some
+    assert fb_0_samples.get_quality(req_0_samples) > fb_2_samples.get_quality(req_0_samples)
+    assert fb_0_samples.get_quality(req_0_samples) > fb_4_samples.get_quality(req_0_samples)
+    assert fb_0_samples.get_quality(req_0_samples) > fb_8_samples.get_quality(req_0_samples)
+    assert fb_0_samples.get_quality(req_0_samples) > fb_16_samples.get_quality(req_0_samples)
+
+
+def test_fbquality_coverage_samples():
+    # Make sure that we get appropriate quality levels for different sample requests
+    
+    fb_2_samples = FrameBufferProperties()
+    fb_2_samples.set_rgb_color(1)
+    fb_2_samples.set_coverage_samples(2)
+    
+    fb_4_samples = FrameBufferProperties()
+    fb_4_samples.set_rgb_color(1)
+    fb_4_samples.set_coverage_samples(4)
+    
+    fb_8_samples = FrameBufferProperties()
+    fb_8_samples.set_rgb_color(1)
+    fb_8_samples.set_coverage_samples(8)
+    
+    fb_16_samples = FrameBufferProperties()
+    fb_16_samples.set_rgb_color(1)
+    fb_16_samples.set_coverage_samples(16)
+    
+    req_0_samples = FrameBufferProperties()
+    req_0_samples.set_coverage_samples(0)
+    
+    req_1_samples = FrameBufferProperties()
+    req_1_samples.set_coverage_samples(1)
+    
+    req_2_samples = FrameBufferProperties()
+    req_2_samples.set_coverage_samples(2)
+    
+    req_4_samples = FrameBufferProperties()
+    req_4_samples.set_coverage_samples(4)
+    
+    req_8_samples = FrameBufferProperties()
+    req_8_samples.set_coverage_samples(8)
+    
+    req_16_samples = FrameBufferProperties()
+    req_16_samples.set_coverage_samples(16)
+
+    # a fb which does not provide the requested number of samples should always 
+    # have a lower quality than another
+    assert fb_2_samples.get_quality(req_4_samples) < fb_2_samples.get_quality(req_2_samples)
+    assert fb_2_samples.get_quality(req_4_samples) < fb_4_samples.get_quality(req_2_samples)
+    assert fb_2_samples.get_quality(req_4_samples) < fb_8_samples.get_quality(req_2_samples)
+    assert fb_2_samples.get_quality(req_4_samples) < fb_16_samples.get_quality(req_2_samples)
+    assert fb_2_samples.get_quality(req_4_samples) < fb_16_samples.get_quality(req_16_samples)
+    assert fb_8_samples.get_quality(req_16_samples) < fb_2_samples.get_quality(req_2_samples)
+    
+    # a fb which has more than the requested samples should have a 
+    # lower quality than one that matches exactly
+    assert fb_2_samples.get_quality(req_2_samples) > fb_4_samples.get_quality(req_2_samples)
+    assert fb_2_samples.get_quality(req_2_samples) > fb_8_samples.get_quality(req_2_samples)
+    assert fb_2_samples.get_quality(req_2_samples) > fb_16_samples.get_quality(req_2_samples)
+    assert fb_2_samples.get_quality(req_2_samples) > fb_16_samples.get_quality(req_8_samples)
+    
+    # the more additional samples the fb provides rather than what is requested, 
+    # the lower the quality should be
+    assert fb_16_samples.get_quality(req_2_samples) < fb_8_samples.get_quality(req_2_samples)
+    assert fb_8_samples.get_quality(req_2_samples) < fb_4_samples.get_quality(req_2_samples)
+    
+    # if the special value of 1 sample is requested, the fb with the highest samples should be 
+    # in favour
+    assert fb_16_samples.get_quality(req_1_samples) > fb_8_samples.get_quality(req_1_samples)
+    assert fb_16_samples.get_quality(req_1_samples) > fb_4_samples.get_quality(req_1_samples)
+    assert fb_16_samples.get_quality(req_1_samples) > fb_2_samples.get_quality(req_1_samples)
+    assert fb_8_samples.get_quality(req_1_samples) > fb_4_samples.get_quality(req_1_samples)
+    assert fb_8_samples.get_quality(req_1_samples) > fb_2_samples.get_quality(req_1_samples)
+    
+    # if 0 samples are requested, the fb with the highest samples should get a reduced quality level
+    assert fb_16_samples.get_quality(req_0_samples) < fb_8_samples.get_quality(req_0_samples)
+    assert fb_16_samples.get_quality(req_0_samples) < fb_4_samples.get_quality(req_0_samples)
+    assert fb_16_samples.get_quality(req_0_samples) < fb_2_samples.get_quality(req_0_samples)
+    assert fb_8_samples.get_quality(req_0_samples) < fb_4_samples.get_quality(req_0_samples)
+    assert fb_8_samples.get_quality(req_0_samples) < fb_2_samples.get_quality(req_0_samples)