I wrote my story about getting some HiDPI patches into LibreOffice, but it was an unfinished one because while the code had gotten accepted into the main Master branch, there was a lot remaining. It hadn’t shipped, I’d only tested it on Gnome and KDE, it hadn’t been tried on Windows, and didn’t work on the Mac. Worst of all, because it missed the December 20 cutoff date for 4.2.0, it was on the default next release train for 4.3.0 in late July.

LibreOffice has a two-part release process. You are encouraged to submit code into the main tree where it can sit for up to 6 months. Twice per year, it is branched and shipped. In between these major releases, every month or so, a new minor release is made containing high priority fixes. Meanwhile most of the development team moves ahead adding features and cleanup for the upcoming release.

The code was too late for 4.2.0, and so it wasn’t getting much feedback. Very few people run the daily builds, with all the monthly releases and release-candidates to test. A friendly chap named Darcy from Australia showed up on the QA alias, built LibreOffice on his Fedora 20 laptop, and verified that it worked, but that was it. The only way to get this code tested was to get it out there.

I had also decided to stop working after the second batch. I had improved many of the most noticeable parts of the product, but I was also steadily making more potential problems for myself. Software isn’t just about code, it is about standing behind your work. I could prove most of my changes were fine, but I was changing places where I didn’t always understand what was going on around. I could justify my fixes, but not the code around it.

Even though LibreOffice is built by a community where other people can fix your bugs, other people can also find your bugs. If your code is causing problems, and no one has time to look into it, it can be reverted. The work is interesting, but I was just a motivated user with some free time during the Christmas holiday. I wanted to reserve time to deal with the inevitable complaints.

In spite of the lack of feedback, since it makes a visual and usability difference, why wait? More computers with these beautiful screens are coming out every day. A very high-resolution screen is the best reason to buy a new laptop. There should be a free software experience that we can enjoy looking at. LibreOffice has plenty of ways to improve, but it can look good in the meanwhile.

Sidebar

The Sidebar was one of the areas I had kept putting off and almost didn’t work on. It is about 70,000 lines of new code providing an alternate UI, and I wanted to focus on the existing one first. I have been using this codebase for almost 10 years and had never missed it. The code was also written by someone from IBM, and so I couldn’t trust that anyone in LibreOffice would be able to help me.

I also hoped that maybe the Apache team would notice bug reports and fix the Sidebar themselves. LibreOffice grabs fixes from Apache on a daily basis. Only about half the changes are useful because in many cases LibreOffice has already done the work, but any pieces of value are ported over, and just a small part of LibreOffice’s churn of the their thousands of improvements per release.

I considered sending an email to the Apache OpenOffice alias asking if they were aware of the problem, and whether they were planning to work on it. It is more efficient to coordinate efforts and not have multiple people re-learning each other’s work. It took me hours to fix problems that could have been fixed over lunch by the person who wrote the code. However, while LibreOffice is taking patches from Apache OpenOffice, the groups are generally not actively planning work, and so asking for HiDPI support for the Sidebar would have been a breach of protocol.

In addition, the codebases are diverging so fixes might not be useful. Apache OpenOffice doesn’t have the OutputDevice::DPIScaleFactor API, so the patches wouldn’t have been directly usable. I didn’t try Apache OpenOffice on Linux, but I did try it on Windows 8.1 and sidebar bitmaps looked doubled. However, Windows 8.1 might have been artificially scaling the entire Apache OpenOffice UI because the text was a blurry mess compared to LibreOffice:

So I could imagine that fixing the Sidebar for LibreOffice would not be a high priority for Apache.

While I personally don’t care about the Sidebar, it is now turned on by default for Impress. I knew that the more places I fixed, the stronger argument I would have to convince people to get the improvements out there. I realized I just needed to motivate myself to learn the code. So while I was in rural northern Michigan with my family over the holiday, there were some quiet nights, and I dug in and learned the Sidebar well enough to fix the major issues. It was the same process and techniques I had used for the other parts of the code. As per usual, finding the correct place to put in a fix was the hardest part.

I’m glad I worked on it because not only did it make the Sidebar fit in visually with the other improvements, it fixed a crashing bug. LibreOffice’s Sidebar is better than OpenOffice’s in that it has dynamic layout when docked. However, the bigger buttons created something wider than the maximum width allowed. With the sidebar starting in an invalid state, the product would still work, but if you tried to resize it with the mouse, you could sometimes get LibreOffice to hang. If you push software beyond its limits, bad things can happen.

I don’t even remember clearly how I found the place to fix, but by just reading enough code, I found the routine SidebarController::RestrictWidth.

@@ -1109,7 +1112,8 @@ void SidebarController::RestrictWidth (sal_Int32 nWidth)

const sal_uInt16 nSetId (pSplitWindow->GetSet(nId));

pSplitWindow->SetItemSizeRange(nSetId,

- gnMaximumSidebarWidth ));

+ gnMaximumSidebarWidth * mpTabBar->GetDPIScaleFactor()));

}

}

Getting on the 4.2.3 release train

Since it was a batch of new code to a stable branch, the LibreOffice Engineering Steering Committee had a discussion about it in one of their weekly meetings:

* HiDPI patches for 4.2.x? (Kendy)

+ LibreOffice does not look good on HiDPI screens at all — close to unusable

+ proposal to merge the HiDPI work to 4.2.x as a late feature

+ I would split the work into ‘safe’ and ‘need real review’ parts & push to a branch

+ can I get ESC approval / 3 independent reviews for that ?

+ is it a feature or a bug-fix ? (Michael)

+ most of the pieces going in enclosed in an if (hidpi) …

+ checking that everything in the right if.

+ less safe part is checking / setting that flag; a small VCL piece.

+ Michael / Caolán signed up to review as/when there is a branch.

Once they gave their support, Kendy prepared for the patches for review.

Working with free software can be fun, but it can also be upsetting because problems can show up at any time. I was happy to see Kendy’s mail to the alias asking for review of the Gerrit patches, but not for very long because Norbert Thiebaud objected to them for the Mac. So after a flurry of emails over 3 days, we eventually got that resolved with another patch. So within a few days after that, the patches got reviewed and into the build for 4.2.3-rc1.

Windows

I was happy to see my efforts finally get into RC build, but not for very long because this screenshot with clipped toolbar buttons showed up in my inbox:

This was upsetting for several reasons. The first is that it could have been found on master months before. Instead, it showed up on a day when I had my own things to work on. But instead of being able to focus on my tasks, I kept thinking about the toolbars, and what was I going to do about it?

I had been wondering for months what would happen when this code was tried on Windows. LibreOffice is created primarily by Linux developers, but Windows is the most popular OS for their users. My machine shipped with Windows but I had wiped it within a few hours and so couldn’t try it. I had used Windows for 15 years, but the last time was 9 years ago and I had no plans to go back.

I had envisioned various possibilities for what might happen on Windows, but cut-off toolbar buttons was not one of them. I also didn’t understand the toolbar layout code. I had written a toolbar manager before and didn’t want to volunteer any of my life on that boring problem.

However, I felt bad and that I should at least put some effort into trying to fix the problem. So that evening, I decided to just read through the toolbar code from beginning to end and see if I found anything obviously wrong. You don’t have to be very clever to notice a bloody knife at a murder scene.

And so I read through toolbox.cxx, including the toolbar layout code, and didn’t notice anything that stood out. So I next went to toolbox2.cxx. About halfway down, I found something suspicious:

/*static*/ Size

ToolBox::GetDefaultImageSize(bool bLarge)

{

const long TB_SMALLIMAGESIZE = 16;

if (!bLarge) {

return Size(TB_SMALLIMAGESIZE, TB_SMALLIMAGESIZE);

} OUString iconTheme = Application::GetSettings().GetStyleSettings().DetermineIconTheme();

return vcl::IconThemeInfo::SizeByThemeName(iconTheme);

}

That 16 was a sign of a place that needed doubling. I didn’t yet know who used that function and whether it would make a difference, but opengrok helped me and found that it was called by the main toolbar layout routine.

So I made a change:

// set defaults if image or text is needed but empty

nDefWidth = GetDefaultImageSize().Width() * GetDPIScaleFactor();

nDefHeight = GetDefaultImageSize().Height() * GetDPIScaleFactor();

However, I couldn’t tell whether it would fix the problem. The comment above the code, while happily in English, explained it was relevant only for empty toolbars. This was not a case I care about.

So I changed the code, and hoped the comment was wrong, but didn’t understand what the implication was. It was a change I would try if I had a Windows box. It requires much less mental effort to test a change than to manually prove what happens in a big function. As long as you can test changes, you can postpone the necessity of full understanding.

So I looked through the code, but didn’t understand it and didn’t want to. On the other hand, it was easy to see the change was reasonable and helpful in several cases. So I submitted it to Gerrit to see if it could get reviewed and approved. If I can sneak it into a daily build, then maybe I could convince someone to try it out. So I submitted and waited for input. 3 days later, Caolán McNamara of Red Hat reviewed and accepted the patch. In fact, Caolán appears to be an email alias that 3 developers are attached to. I personally think Arch is a better distro for me than Fedora, but I am very grateful for the useful investments that Red Hat is making.

Once my patch got into the daily builds, I emailed the tester asking if he could try one out, but he replied that he didn’t have time. So I was stuck and frustrated again. I couldn’t know if the change fixed the bug, which made me uncomfortable to ask for a patch to be triple-reviewed and back-ported. Blind-fixes to stable branches are generally not a good way to work.

I also didn’t know how to submit changes to anything other than the master branch. The LibreOffice wiki is great for new developers, but didn’t cover that specific topic. So I decided to ask Caolán if he could submit them to Gerrit to get them in before the 4.2.3 release. Caolán did that and even sent me the Git incantations so I can do it in the future.

With some reviews from Norbert and Miklos Vajna, the patch got into the 4.2.3 branch. So I was very happy, but not for very long. Because I soon noticed that the final 4.2.3 RC build had already been made. You can still push changes to the Git branch, but it doesn’t matter, the digital equivalent of air guitar. It is possible to mark bugs as release critical and delay the release, but this bug didn’t meet the bar. I should just have been happy that the bug was likely now fixed, but instead I was bummed it had missed the train by a few hours.

However, the Heartbleed bug and a few other important ones showed up, and so another RC was made. I’m probably the only person on the Internet, other than the NSA, who was happy for Heartbleed.

KDE Regression

Just as LibreOffice 4.2.3 was shipping, another bug showed up from a KDE Ubuntu user:

Someone with a 1920x1080 15.6” monitor was seeing the HiDPI mode kick in. This is a bad bug because it is a regression. The goal of the feature is to improve the experience for HiDPI users, not break it for everyone else. Degrading a product for other people is the fastest way to get your code reverted.

This would have been stressful for me, but several weeks earlier I had studied the Linux DPI detection code and multi-monitor support. Since I knew exactly what 4.2.3 was doing here, I didn’t worry about being able to quickly solve the problem. I just needed to figure out what DPI data LibreOffice was getting from the OS. You can stare at code as much as you want, but if you depend on hardware-specific information, you can’t prove it correct until you test on other computers.

I had taken the time to learn the code because I submitted a patch to LibreOffice for 4.3 that simplifies it to only fetch from xrdb and never bother to fetch from X Windows. I had found the information unreliable for my laptop. On my machine, X.Org tells me it is 96 DPI on a 33” by 18” monitor. It is pretty impressive to squeeze all that real estate into a 13.3” screen. The patch to ignore X isn’t in 4.2.x, but that wasn’t a problem because the hard part is understanding the code. The tester was very helpful in quickly giving me the information I needed.

The problem is simple to describe. The monitor was 141 DPI, but X said it was 139x144. Of course it is a mess with bad data of different DPI values in the X and Y direction, but that was irrelevant here. The issue was that LibreOffice’s doubling kicked in at 144 DPI in the Y direction:

mnDPIScaleFactor = std::max((sal_Int32)1, (mpWindowImpl->mpFrameData->mnDPIY + 48) / 96);

144 + 48 == 192 / 96 == 2

I could think of several fixes, but I wasn’t sure what was best, so I decided to ask Kendy who had written that line of code. Within a few days, he submitted an improvement that will not cause this new mode to kick in until at least 168 DPI. That fixes the problem for this machine, and hopefully others. So with these fixes, things are in decent shape. The next issue is Unity, which is currently broken.