Support fractional device pixel ratio in native Qt widget

When fractional DPI scaling is used, the VTK render window / interactor has the wrong size because it's calculated from devicePixelRatio() which is an integer API returning a rounded value.

Starting with Qt 5.6, there's a devicePixelRatioF() API that can return a non-integer device pixel ratio. This makes sure the new API is used in QVTKOpenGLNativeWidget, when available.

The problem was first reported by me on the VTK developers mailing list:

https://www.vtk.org/pipermail/vtk-developers/2018-February/035826.html

Test Program (Sphere.cxx)

#include <vtkSphereSource.h>
#include <vtkPolyData.h>
#include <vtkSmartPointer.h>
#include <vtkPolyDataMapper.h>
#include <vtkActor.h>
#include <vtkRenderWindow.h>
#include <vtkRenderer.h>
#include <vtkRenderWindowInteractor.h>
#include <vtkGenericOpenGLRenderWindow.h>

#include <QApplication>
#include <QPointer>
#include <QSurfaceFormat>

#include <QVTKOpenGLNativeWidget.h>

int main(int argc, char *argv[])
{
  QSurfaceFormat::setDefaultFormat(QVTKOpenGLNativeWidget::defaultFormat());

  QApplication app(argc, argv);

  vtkNew<vtkGenericOpenGLRenderWindow> window;
  QPointer<QVTKOpenGLNativeWidget> widget = new QVTKOpenGLNativeWidget();
  widget->SetRenderWindow(window.Get());

  // Create a sphere
  vtkSmartPointer<vtkSphereSource> sphereSource = 
    vtkSmartPointer<vtkSphereSource>::New();
  sphereSource->SetCenter(0.0, 0.0, 0.0);
  sphereSource->SetRadius(5.0);

  vtkSmartPointer<vtkPolyDataMapper> mapper = 
    vtkSmartPointer<vtkPolyDataMapper>::New();
  mapper->SetInputConnection(sphereSource->GetOutputPort());

  vtkSmartPointer<vtkActor> actor = 
    vtkSmartPointer<vtkActor>::New();
  actor->SetMapper(mapper);

  vtkSmartPointer<vtkRenderer> renderer = 
    vtkSmartPointer<vtkRenderer>::New();
  window->AddRenderer(renderer);

  renderer->AddActor(actor);

  widget->resize(300, 300);
  widget->show();

  return app.exec();
}

CMakeLists.txt

cmake_minimum_required(VERSION 2.8)
 
PROJECT(dpibug)
 
find_package(VTK REQUIRED)
include(${VTK_USE_FILE})
 
add_executable(Sphere MACOSX_BUNDLE Sphere.cxx)
 
if(VTK_LIBRARIES)
  target_link_libraries(Sphere ${VTK_LIBRARIES})
else()
  target_link_libraries(Sphere vtkHybrid vtkWidgets)
endif()

Output Before (Qt 5.9.5, run with QT_SCREEN_SCALE_FACTORS=1.4 ./Sphere)

Notice how the render window has the wrong size.

Output After (Qt 5.9.5, run with QT_SCREEN_SCALE_FACTORS=1.4 ./Sphere)

The render window is now the correct size.

Warnings:

• the merge request is marked as a work-in-progress.

The warnings do not need to be fixed, but it is recommended to do so.

This was a long time in the making, with a number of iterations in different branches. I was just looking at the commit. I vaguely remember some testing we did switching from retinal to non-retinal displays with a two monitor set up on a Mac. I think that code may have been working around a bug we were seeing where the resize event from retinal to non-retinal wasn't firing so we were effectively toggling the size (double or halve it), but if that were the case it could use a comment, but the commit message mentions it (916dad14):

This enables retinal Mac to work, there is a little extra work to
account for the pixel ratio not triggering a full resize event as is
expected, and a question of whether we need to invoke a configure event.

Some small concerns - if we are using double/float then do we need to use a fuzzy compare?

@mhanwell Aha alright. I have a Mac next to me, I'll try to make a build on it to test a Retina/non-Retina multi-monitor setup. I have it on my TODO to test that anyway, because our CEO will be demoing our app on exactly such a setup soon.

@mhanwell I don't think a fuzzy compare should be needed. This if (ratio == 1) else if (ratio == 2) { ... looks strange to me anyway, since even when the pixel ratio API in Qt was all integers (Qt < 5.6), there was never any guarantee that it'd be only 1 or 2. It could be 42 (though that would be a strange setup...).

So let me test on a dual-monitor Retina setup if we can drop the if/else comparing to ratio altogether. If so, we don't even have to think about fuzzy comparisons :)

FWIW: When working on Mac Retina support months ago, I did test multi-monitor Retina & non-Retina and IIRC it works correctly. But that was without Qt.

You are right actually, if the comparisons go away then fuzzy comparison is obviously not needed. This was all based on the Qt event system, and the events we had to respond to within that. If you support any arbitrary scaling factor, then the doubling/halving hack will not help anyway. I had meant to circle back, and see if there was an event we were missing for the retina to non-retina (or vice-versa) switch.

added 1 commit

• 9289ff42 - Cast explicitly in SetSize and use a tolerance

Compare with previous version

Warnings:

• the merge request is marked as a work-in-progress.

The warnings do not need to be fixed, but it is recommended to do so.

This may be superseeded by !3745 (closed) , wich will replace QVTKOpenGLWidget anyway. @jpouderoux @LucasGandelKitware

mentioned in merge request !3745 (closed)

@mwestphal Indeed. BTW is that on track to being merged in time for 9.0 do you think?

We are actively working on it and hope to merge in april/may.

@mhanwell Now I see that the new QVTKOpenGLWidget @mwestphal mentioned is merged (!4317 (merged)), and that it does switch to float (which is good). But it still has that strange special-casing of ratio == 1 and ratio == 2 in QVTKInteractorAdapter::SetDevicePixelRatio which I don't quite understand (and came across while doing this old MR).

I believe special-treating 1 and 2 like that could interfere with systems with non-integer scaling factor? (e.g. 1.5, 1.4).

I agree @estan, the integer case may have worked, but the floating point case is unlikely to follow that logic.

@estan I suppose that with new implementation, QVTKInteractorAdapter code for ratio==1|2 is actually useless because this->GetInteractor()->SetSize(deviceSize.width(), deviceSize.height()); is called in QOpenGLVTKWindow after every call to QVTKInteractorAdapter::SetDevicePixelRatio... @LucasGandelKitware Do you agree?

@jpouderoux Ah yes. Looking at it, I think that was sort of the case also in the old one as well (what is now QVTKOpenGLSimpleWidget). It does call QVTKInteractorAdapter::SetDevicePixelRatio once in the constructor, and once in the recreateFBO function, and in the latter, the call is immediately followed by:

  if (vtkRenderWindowInteractor* iren = this->RenderWindow->GetInteractor())
  {
    iren->SetSize(deviceSize.width(), deviceSize.height());
  }

But I guess for users from outside VTK calling that SetDevicePixelRatio, the 1/2 logic there should be fixed? Though what application would want to call that?

So I guess my question is: Should I make a new MR with only the changes to QVTKInteractorAdapter? What about the tolerance in the truncation of the device pixel ratio-scaled window size? Should I include those changes as well? (Note that I added the same kind of tolerance to QVTKOpenGLWidget in this MR, so if I should include that, a new MR would add that to the new QVTKOpenGLWidget/QVTKOpenGLWindow).

@estan @jpouderoux QVTKInteractorAdapter code for ratio==1|2 is indeed useless because the widget will set the interactor size anyway. This 1/2 logic should be fixed now that we use float values. Not sure how it would impact other application using QVTKInteractorAdapter, so it might be better to split MRs.