sbragin (Sergio Bragin)
User

Projects

User does not belong to any projects.

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Wednesday

  • Clear sailing ahead.

User Details

User Since
Feb 7 2018, 8:48 PM (32 w, 4 d)
Availability
Available

Recent Activity

Mar 6 2018

sbragin added a comment to D11074: Fix opening files via a file manager on Mac.

No, none of the Qt APIs you're using are Mac-specific, they just do nothing on other platforms (you won't get any QFileOpen events). Not using #ifdefs also means that if ever Qt extends the API to other platforms (MS Windows, for instance) Okular will leverage it without further changes.

I haven't looked in detail at the additional goodies you added compared to my bare-bones implementation; could it have or be given a more general interest?

Mar 6 2018, 8:40 PM · Okular
sbragin added a comment to D11075: Make Okular show the list of opened windows in the Dock menu.

Qt4 apps used to create a temporary NIB file in $TMPDIR and load that. I cannot recall having seen something similar in Qt5 and cannot find any evidence of it in the code. But just as with Info.plists there might be a way to create the information normally read from a nib/xib file purely in memory.

Mar 6 2018, 8:36 PM · Okular
sbragin added a comment to D11074: Fix opening files via a file manager on Mac.

There is no specific reason to hide the QFileOpenEvent handling in a Mac-specific file and behind #ifdefs . I seem to recall that the same event mechanism was used on another, now defunct Qt platform. Other KDE applications (notably Kate) don't bother and just include the extra code unconditionally.

Mar 6 2018, 6:48 PM · Okular
sbragin added a comment to D11075: Make Okular show the list of opened windows in the Dock menu.

I'm not sure if all Mac applications always show the open documents under their Dock tile (that menu can be used for anything, of course),

Mar 6 2018, 6:32 PM · Okular

Mar 5 2018

sbragin added a comment to D11074: Fix opening files via a file manager on Mac.

Does each and every macOS application which would like to open a file via a file manager need to implement the same macapplication.cpp (or so) again and again (which makes use of a QFileOpenEvent, yes, but that's not the point; the point is about the content of macapplication.cpp, which is not Okular-specific).

Mar 5 2018, 9:53 PM · Okular
sbragin added a dependency for D11075: Make Okular show the list of opened windows in the Dock menu: D11074: Fix opening files via a file manager on Mac.
Mar 5 2018, 9:48 PM · Okular
sbragin added a dependent revision for D11074: Fix opening files via a file manager on Mac: D11075: Make Okular show the list of opened windows in the Dock menu.
Mar 5 2018, 9:48 PM · Okular
sbragin added a comment to D11074: Fix opening files via a file manager on Mac.

Let me rephrase: each and every macOS application which needs to open file via a file manager needs to implement this same code?

Mar 5 2018, 9:45 PM · Okular
sbragin added a comment to D11074: Fix opening files via a file manager on Mac.

Isn't this another case of "should be fixed in QApplication or another lower level library"?

Mar 5 2018, 9:40 PM · Okular
sbragin requested review of D11075: Make Okular show the list of opened windows in the Dock menu.
Mar 5 2018, 9:36 PM · Okular
sbragin requested review of D11074: Fix opening files via a file manager on Mac.
Mar 5 2018, 9:16 PM · Okular

Feb 25 2018

sbragin added a comment to D10808: Implement the generation of a custom Info.plist for the Mac OS bundle.

Adding supported document types in the plist has zero interest if you don't modify the code so it catches Qt's FileOpen events

Feb 25 2018, 2:25 PM · Okular
sbragin added a comment to D10808: Implement the generation of a custom Info.plist for the Mac OS bundle.

The task at hand seems more complex and I have my doubts as to how worth it would be the effort to implement support for doing this in the ECM. I'd say there are Mac-specific things that should be higher on the list.

Feb 25 2018, 2:19 PM · Okular
sbragin added a comment to D10808: Implement the generation of a custom Info.plist for the Mac OS bundle.

I think that this was already discussed, but would it make sense to have a function in extra-cmake-modules which generates the plist file based on some parameters (known from somewhere else or passed explicitly) instead of hand-writing that file?

Feb 25 2018, 12:03 PM · Okular
sbragin added a comment to D10808: Implement the generation of a custom Info.plist for the Mac OS bundle.

I think that this was already discussed, but would it make sense to have a function in extra-cmake-modules which generates the plist file based on some parameters (known from somewhere else or passed explicitly) instead of hand-writing that file?

Feb 25 2018, 11:56 AM · Okular
sbragin added a comment to D10808: Implement the generation of a custom Info.plist for the Mac OS bundle.

Adding document-type icons isn't strictly necessary. The Finder will display documents with the icon of the default application configured to open the type in question, or with the application configured for the individual document. Applications that provide icons for the different document types they support usually do this by adapting the application icon instead of using a generic icon for the document type (exceptions aside of course). (I'm not at my Mac right now and hardly use the Finder in practice; I wouldn't be surprised if the Finder actually tagged application icon with a standard document type representation when apps don't provide their own icons.)

Feb 25 2018, 11:47 AM · Okular
sbragin added a comment to D10808: Implement the generation of a custom Info.plist for the Mac OS bundle.

This is wrong because lots of the support for those file formats depends whether you have the backends or not, so this either should be made to depend on which dependencies where found when running cmake or each backend plugin should install its own file (if macos supports that).

Feb 25 2018, 11:40 AM · Okular

Feb 24 2018

sbragin added a comment to D10808: Implement the generation of a custom Info.plist for the Mac OS bundle.

Comment #1: Icons for file types, associated with Okular, are missing. It has been left for a future patch.
Comment #2: Not all supported formats have been added to the Info.plist file.

Feb 24 2018, 8:04 PM · Okular
sbragin added a reviewer for D10808: Implement the generation of a custom Info.plist for the Mac OS bundle: Okular.
Feb 24 2018, 8:01 PM · Okular
sbragin requested review of D10808: Implement the generation of a custom Info.plist for the Mac OS bundle.
Feb 24 2018, 7:59 PM · Okular

Feb 18 2018

sbragin added a comment to D10415: Fix realDpi function for Mac.

So should i commit this or is there a better patch coming? (yes, i didn't really read all those posts)

Feb 18 2018, 6:11 PM · Okular

Feb 11 2018

sbragin added a comment to D10415: Fix realDpi function for Mac.

Did you verify the actual size at which elements are shown? If so maybe you can your test document and protocol so I can verify this on 10.9 (and maybe have it verified on 10.13 by one of my users)?

So, I just created a simple app, and tested the part of the code in question only. The behavior for the Qt part was the same as in Linux. I also wrote a sample, which uses methods from Apple frameworks only (CGDisplayScreenSize and CGDisplayBounds), it gave the same result as the Qt code

Feb 11 2018, 3:43 PM · Okular
sbragin added a comment to D10415: Fix realDpi function for Mac.

Do we agree that my patch is the minimum way of achieving the same thing your patch does? Not that I want to be lazy, but if it is I can already confirm that I have not noticed any issues with using the standard Utils::realDPI function.

Feb 11 2018, 2:53 PM · Okular
sbragin added a comment to D10415: Fix realDpi function for Mac.

No, I meant: if the logical variant works for us here, the physical variant should work fine, too.
That they not give you the same results in all cases is clear, given they are logical vs. physical.

Ok, got it.

Feb 11 2018, 1:15 PM · Okular
sbragin added a comment to D10415: Fix realDpi function for Mac.

On a sideways related note:
I have at least one patch that improves Okular/Mac integration, allowing it to act as an alternative or complement to the native Preview.app .

I've never made the effort to assess and polish them up for upstreaming because I rarely use Okular on Mac, but if someone wants to cherry-pick they're here:
https://github.com/RJVB/macstrop/tree/master/kf5/kf5-okular/files

Feb 11 2018, 1:10 PM · Okular
sbragin added a comment to D10415: Fix realDpi function for Mac.

It does work. The old one uses deprecated methods, doesn't work properly and can not be even compiled, in fact.

How so? I have been building Okular on Mac for a long time (IIRC since before the master branch was KF5 based). I'm on Qt 5.9.3 and OS X 10.9.5 and the only change I currently make to utils.cpp is:

Hi René! Nice to see you here! Apart from warnings and the errors, that you had fixed in your patch also, the old version was giving me some crap, while I was testing non-native resolutions. I don't have 10.9 at hand, so, it would be good if you could check that part of Linux/pure Qt code. I confirm that it works with 10.11 and 10.12.

Feb 11 2018, 1:05 PM · Okular
sbragin added a comment to D10415: Fix realDpi function for Mac.

In our company we use the logicalDpiX()/logicalDpiY() on the widget we paint to, that works on macOS, too.

I assume the physicalDotsPerInchY() should work the same.

Feb 11 2018, 12:54 PM · Okular
sbragin updated the summary of D10415: Fix realDpi function for Mac.
Feb 11 2018, 12:47 PM · Okular

Feb 9 2018

sbragin added a comment to D10415: Fix realDpi function for Mac.

Works with Qt 5.8.0.

Feb 9 2018, 9:31 PM · Okular
sbragin added a comment to D10415: Fix realDpi function for Mac.

I have tested against Qt 5.9.3 and 5.10 on Mac OS 10.12.

Feb 9 2018, 8:23 PM · Okular
sbragin added a comment to D10415: Fix realDpi function for Mac.

Can you please expand the commit message? Is the old code not required anymore because Qt now "just works"? Does it apply also to the minimum Qt versions required by Okular, on all the supported versions of macOS?

It does work. The old one uses deprecated methods, doesn't work properly and can not be even compiled, in fact.

Feb 9 2018, 8:14 PM · Okular
sbragin updated the summary of D10415: Fix realDpi function for Mac.
Feb 9 2018, 8:12 PM · Okular
sbragin updated the summary of D10415: Fix realDpi function for Mac.
Feb 9 2018, 8:11 PM · Okular
sbragin requested review of D10415: Fix realDpi function for Mac.
Feb 9 2018, 8:08 PM · Okular