The mess that is the VCL

 Let me count the ways, in no particular order and in no way exhaustive:

• OutputDevice is the base class for printing, windowing and PDFs. It doesn't just do output. 
• OutputDevice has GetOutDevType() because the base class needs to know what child class is using it. Ugh. 
• OutputDevice drawing primitives not only draw, but they record a metafile. There are literally functions that turn off drawing and just let it record the metafile. I made an attempt at seperating the concerns, but it got nowhere. 
• VCL relies on DrawingLayer and DrawingLayer relies on the VCL. 
• There is a concept of a VirtualDevice, which is derived from OutputDevice. VirtualDevice does a bunch of things, but one of which is alpha-handling. In OutputDevice, there is a member which is a VirtualDevice. Each drawing function in Outputdevice calls upon the correlated drawing function in this member VirtualDevice.
  
• Bitmaps don't get modified via the Bitmap class. Instead, you have to use BitmapInfoAccess, BitmapReadAccess and BitmapWriteAccess. I'm still puzzling out why these are seperate classes. 
• Bitmaps are transformed in SalGraphics indirectly via OutputDevice. Except when they aren't, in which case it fails, whereby OutputDevice tries an alternative way via SalGraphics. Otherwise, it tries its own poor man approach at drawing the bitmap. Consequently, often times you bypass the platform optimized ways of doing things, because its not been implemented.
  
• Fonts are lazy loaded from OutputDevice. There is no central font manager. To get the fonts, you have to go through SalGraphics. To get a SalGraphics, you need to initialize a lot of stuff not related to fonts. 
• Font caching is done from OutputDevice. Lazily. Font data is updated for all frames. Frames are a concept needed for Windows. Frames are not a concept needed by Printers and VirtualDevices, or even PDFs. Note that Printers, VirtualDevices and PDFs all inherit from OutputDevice. 
  
• OutputDevice converts between "logical" units and display units. It's a nightmare to know what each function needs what sort of units. For the mapping between units, I refer you to vcl/source/gdi/mapmod.cxx and vcl/source/outdev/map.cxx
• There is tools and basegfx. They do the same thing, though basegfx is considerably better written. You have Size and B2DSize, Point and B2DPoint, Polygon and B2DPolygon, PolyPolygon and B2DPolyPolygon. OutputDevice must handle it all. 
• Gradient handling is sort of half baked in OutputDevice, much of gradient handling is done in other modules. 
  
• Font substitution is truly, truly weird. PhysicalFontSelect::FindFontFamilyByAttributes() has clearly got a bug in it - (e.g. ImplFontAttrs::None == ((nSearchType ^ nMatchType) & ImplFontAttrs::Rounded an XOR?) and it is a truly strange weighting scheme. Yes, I did try to untangle that beast with proper unit tests, but gave up after being told I was being unreasonable. 
• There is VCL, canvas, cppcanvas and drawinglayer. drawinglayer is way better than VCL, but we are stuck with VCL for everything. 
• Consider the following Window hierarchy: WorkWindow inherits from SystemWindow, which inherits from Window. Window holds an OutputDevice to do stuff. WindowOutputDevice derives from OutputDevice. This is needed because OutputDevice often needs to know if it is doing Window operations, via WindowOutputDevice. Try untangling this in your head.
  
• Text layout is its own beast, and has its own set of classes. A lot of text layout is worked out in OutputDevice. 
  
• Text layout is done via OutputDevice::ImplLayout(). I present to you the ImplLayout function signature:
  
  

      std::unique_ptr<SalLayout> ImplLayout(
          const OUString&, sal_Int32 nIndex, sal_Int32 nLen, const Point& rLogicPos = Point(0, 0),
          tools::Long nLogicWidth = 0, KernArraySpan aKernArray = KernArraySpan(),
          std::span<const sal_Bool> pKashidaArray = {}, SalLayoutFlags flags = SalLayoutFlags::NONE,
          vcl::text::TextLayoutCache const* = nullptr, const SalLayoutGlyphs* pGlyphs = nullptr,
          std::optional<sal_Int32> nDrawOriginCluster = std::nullopt,
          std::optional<sal_Int32> nDrawMinCharPos = std::nullopt,
          std::optional<sal_Int32> nDrawEndCharPos = std::nullopt) const; 

Separating metafile processing from rendering in OutputDevice

The OutputDevice class is what we use for rendering a whole host of different things, and other classes inherit from it to do their rendering. Notably a little while ago Noel Grandin separated OutputDevice from Window, which is a huge boon for the codebase!

In an attempt to further separate the concerns of OutputDevice, I noticed a long time ago that not only does it do rendering, but it records actions in a metafile. These are two related and yet quite separate things. My goal is to shift out the rendering into a RenderContext class. I have already got a PoC on GitHub, which has already done much of the work, albeit in a messier way (basically it was a test to see if this was even feasible).  

Obviously migrating rendering functionality into its own class is not something to take lightly, so I have been writing smaller patches that test out the code. When I say "small", I really mean this. Currently I am testing out a bunch of code that sets state in OutputDevice, which can be found in the outdevstate topic branch in gerrit. 

Along with test the state functions of OutputDevice, I am also adding some tests to the bitmap functionality of OutputDevice. I recently landed a patch to move GetDownsampledBitmap() out of OutputDevice and into BitmapTools.cxx because it really didn't need to completely rely on OutputDevice. This bit of code will hopefully one day migrate it's way into the VCL backend modules. You can monitor the progress I'm making via the bitmap topic branch in gerrit. 

I have to say that writing tests is not always easy. Often there is no easy way of accessing the functions, especially if they are private, but luckily most of the functions are protected so I can at least write a mock class to expose these functions without having to modify the code.

Currently I have the following topic branches:

• outdevstate
• bitmap
• text
• font
• animation
• image
• printer
  

Drawing in OutputDevice

For a long time now I have noticed that OutputDevice is a class that is tightly coupled to drawing primitives such a pixels, lines, rectangles, etc. To draw new primitives in OutputDevice, you need to change the interface by adding another function, often you need to add new private functions, etc.

I have never been entirely comfortable with this - I believe that we shouldn't vary the OutputDevice class, but instead the functionality should be implemented in a command pattern. In a command pattern, you use an object to encapsulate the functionality used to perform an action. What this means is that OutputDevice no longer needs to know how to directly draw a line, pixel, rectangle or any other primitive we throw at it - this is all done in the command object. I call these OutputDevice Drawables. It turns out, I find it easier to test a command object.

In fact, the more I rewrote code to use the command pattern, the more I found that we were using a particular sequence of actions in most of our drawing functions:

1. Add action to GDI metafile 
2. Can we draw? No - exit 
3. Initialize the clipping region if possible 
4. Initialize the object's color 
5. Initialize the object's fill color 
6. Acquire the SalGraphics instance 
7. Do the actual drawing! 
8. Paint alpha virtual device

When I realised this, I decided to follow the principle of encapsulating what was varying - point 7 - the actual drawing. Everything else was the same - at least so I initially thought. It turns out this is not the case all the time, so I allowed two things - a function that can be overridden that tells the Drawable that the step should be bypassed, and in the case where we need to through all this scaffolding out the window a way of bypassing it entirely. However, this is an example of a template method pattern.

You can see the base work for this in a gerrit patch I submitted today. To use the actually command pattern is very easy: if there is not an existing function like DrawPixel in OutputDevice, you just have to invoke the command object via OutputDevice::Draw(Drawable* pDrawable). An example can be found in the unit test.

To implement a new command, you just derive from Drawable - the simplest Drawable class you can define only needs a constructor with the parameters you would normally send to a Draw command in OutputDevice (e.g. PixelDrawable only needs a Point sent to the constructor), and you implement the actually drawing in DrawCommand(OutputDevice* pRenderContext).

A few things I found helped me when I created Drawables - I quite like passing parameters to functions, I'm not a huge fan of a lot of state in the class. Drawables work because they essentially work by calling the Execute function, this handles all the basic drawing for you on the state you set in the constructor. I have found in some patches that I have on github that the functionality is somewhat complex, for this I have extract function to simplify test and readability of the code (an example of this is GradientDrawable - see GradientDrawable.cxx and GradientDrawableHelper.cxx).

Netflix don't know their own security processes

On the 9th February, unbeknownst to me, my wife accidentally typed in the wrong password several times into Netflix.

I then got an email from Netflix:

Password reset required

Dear CHRISTOPHER,

We’ve detected a suspicious sign-in to your Netflix account. Just to be safe we've reset your password and you’ll need to set a new one.

SET A NEW PASSWORD

Use the button above or type www.netflix.com into your browser, click on “sign in”, and then click “forgot your email or password.” Follow the instructions to set a new password.

If you have any questions we are here to help. Visit the Help Center for more info or contact us.

–The Netflix Team

I spoke to 4 seperate people, and they all say that this is a "phishing email" as there is no way that Netflix can ever reset the password from their end - apparently only customers can initiate password resets.

I got pretty annoyed as I take security fairly seriously, and if someone had compromised my account, I'd like to know more about it. So I test what I'm being told by Customer Service, and find the following link:

https://help.netflix.com/en/node/56461

This reads:

We will occasionally email our members encouraging them to change their account passwords as a precautionary measure. These emails are usually sent in response to username and password breaches at other companies, phishing schemes, observed suspicious account behavior, or malware attacks.

In these instances, we will notify you that we have reset your password; however, some of the devices you own may stay signed in for your convenience. You will have the option to automatically sign out of all devices when you set up a new password. If you used the same password for any other online sites, we recommend that you also change your password for those accounts.

So the last guy I speak to talks over the top of me, and says "you just don't get it". So I ask for a supervisor. I wait for 40 minutes (I'm quite a determined guy) and whilst I'm waiting I go online. The following is the chat transcript

tl;dr basically "Jason" tells me that they never initiate password resets and that I'm looking at a phishing email (which I'm not, and I prove by sending him the message headers), that I'm reading the Netflix article "out of context" (???), and he will ask to have it updated.

Your issue is: rude and inaccurate customer service

You are now chatting with: Jason

You

hi Jason

Netflix Jason

Hi! :)

Netflix Jason

I'm sorry to hear that.

You

I'm on the phone right now

Netflix Jason

On behalf of Netflix please forgive us for that experience.

You

four customer service reps have now told me it is impossible for me to get an email saying due to suspicious activity Netflix reset my password

You

one of them was a supervisor

Netflix Jason

I just hope a resolution was provided for the concern you're calling about.

You

who just talked over the top of me, didn't know what he was talking about and accused me "you just don't get it"

You

no, I'm still on the line!

You

what, precisely, don't I get?

You

they insisted it must have come from "hackers" or someone hit the reset password link

You

and it didn't

You

is this the sort of customer service Netflix gives?

You

that's literally four reps who don't believe Netflix can change my password

You

https://help.netflix.com/en/node/56461

Netflix Jason

Can you tell me exactly what happened?

You

I even got the last guy to read it, and he STILL didn't believe this could happen

Netflix Jason

Actually we can't.

Netflix Jason

We don't have access.

You

actually, you can't but Netflix can

Netflix Jason

We can only send you a reset link.

You

the email is "Dear CHRISTOPHER,
We’ve detected a suspicious sign-in to your Netflix account. Just to be safe we've reset your password and you’ll need to set a new one."

You

so yes, Netflix can and will reset passwords

Netflix Jason

We don't even have your passwords in our system.

You

it even says so on your site:

Netflix Jason

We don't keep records of passwords.

You

In these instances, we will notify you that we have reset your password; however, some of the devices you own may stay signed in for your convenience. You will have the option to automatically sign out of all devices when you set up a new password. If you used the same password for any other online sites, we recommend that you also change your password for those accounts.

You

https://help.netflix.com/en/node/56461

Netflix Jason

That notification is totally taken out of context.

You

then why did Netflix send me an email telling me you had reset my password?

You

and sure enough, my password was reset

Netflix Jason

The customer will have to initiate the password change.

You

wrong

You

no, read that article on the Netflix website again!

Netflix Jason

Then the system or representative will send a link to your email so that you can change the password.

You

it says "In these instances, we will notify you that we have reset your password"

You

in other words, Netflix resets the password

Netflix Jason

We can't really.

You

can you explain how your own website says that?

You

yeah, then why does your own website say that Netflix can reset my password?

You

https://help.netflix.com/en/node/56461

You

have you read this?

Netflix Jason

Again it's been taken out of context.

You

how?

You

I got an email that my password has been reset

Netflix Jason

If you want I will provide feed back for you so that it can be revised.

You

and indeed, my password had been reset

You

it doesn't need to be revised!

You

that's what actually happened!

You

why did I get that email then?

Netflix Jason

Look, you made contact. You need assistance. I'm telling you how it works.

Netflix Jason

Then let's figure it out.

You

OK, if you are telling me how it works, then can you explain the email I received?

Netflix Jason

Have you lost access to your Netflix?

You

the email says:

You

"Password reset required
Dear CHRISTOPHER,
We’ve detected a suspicious sign-in to your Netflix account. Just to be safe we've reset your password and you’ll need to set a new one.
SET A NEW PASSWORD
Use the button above or type www.netflix.com into your browser, click on “sign in”, and then click “forgot your email or password.” Follow the instructions to set a new password.
If you have any questions we are here to help. Visit the Help Center for more info or contact us.
–The Netflix Team"

You

how is that "out of context"?

Netflix Jason

Ok this is clearly a phishing email.

Netflix Jason

We do not send anything like that.

Netflix Jason

Test your account.

Netflix Jason

SIGN OUT then SIGN IN. See if you have lost access.

You

nope, mail headers show it's from Netflix

You

Delivered-To: <my email address>
Received: by 10.74.131.83 with SMTP id q19csp1869842oog;
Thu, 8 Feb 2018 13:11:40 -0800 (PST)
X-Google-Smtp-Source: AH8x227dnJIPEgqm2ezeY/9ZE3fAMeUmiYW42h0V/T2YOyFJMEzfnNE2+rIv9y58XO4qxaPrB4wd
X-Received: by 10.55.214.65 with SMTP id t62mr536593qki.351.1518124300356;
Thu, 08 Feb 2018 13:11:40 -0800 (PST)
ARC-Seal: i=1; a=rsa-sha256; t=1518124300; cv=none;
d=google.com; s=arc-20160816;
b=xvZttTVW9UR+sy6tboHlwcnWhwgPIkCwg/pzu7Jku2nT0cVkX4OVqXHNAXbY79Ld1y
6xDIePeWfKnVHid+uT2ww8VPi1M4zzVleb8ofeyIcWRN3c8ySScC77jejsAUX4NbYDTK
BUi4c+fHTkTvIgixnLNTHZE3S4jeyrdFsJsxX/gadhzvf4Pt1p72HIngPAh/iwB056rZ
F2a+2pNzfRbe0XWy78rcs1x/jvjqGnRKz4CHg3blVgBkgPi8ocKh/8F+7futS2QLsdYi
C2HuvUfXoE6hw3GJLfWAP/ltJini861Xv/Wdf0IDsqC17bPbb6UXDXZCd89EpyuvvFV8
UjbA==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816;
h=feedback-id:list-unsubscribe:precedence:mime-version:subject
:message-id:to:sender:from:date:dkim-signature:dkim-signature
:arc-authentication-results;
bh=oOAy3CiUxh+PbdPyEubLiydYu/fgIyh2XEBmyx49D7A=;
b=TCbuJ7coONxCi5puG0qI7B7SGcR9FywSxGm5b03YqPJ7WppaTQNTT/VOu89q/FYqcu
/8mr50OFwA2B1p9mzlJiAJvc1H44bs40RGqykoLb4wH/rTuQXl6YQLArlU5pNkuclK1v
6aCCH/Zn8Cthpf4MaR4k8wLuz18NX8gwfSaOvm6NS8sRcDvfyNEZwWDD4wfvA8ui7xbZ
o7N2OQEeu50VH9e0KE9AP+/iYVndI0pb3o+GnPGgnB7kHoNoiy95qw4Y8r0P4BrSeF7o
7Lgh0DBB9RsIbh/vJgkw4t966fqn4C5JfsVmrzHTpVIusgzESqUHVNr9r2tr0K82lFAT
Xf2A==
ARC-Authentication-Results: i=1; mx.google.com;
dkim=pass header.i=@netflix.com header.s=wsh2ycm5iultkbdysii4ijqi22uurzvv header.b=Ju48Kj89;
dkim=pass [non-Netflix link blocked]header.s=224i4yxa5dv7c2xz3womw6peuasteono header.b=avCLQKcv;
spf=pass (google.com: domain of 010001617742739c-0daddaa6-5755-4638-ba13-3efbdad23cf8-000000@mailer.netflix.com designates 54.240.14.187 as permitted sender) smtp.mailfrom=010001617742739c-0daddaa6-5755-4638-ba13-3efbdad23cf8-000000@mailer.netflix.com;
dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=netflix.com
Return-Path: <010001617742739c-0daddaa6-5755-4638-ba13-3efbdad23cf8-000000@mailer.netflix.com>
Received: from [non-Netflix link blocked] [non-Netflix link blocked] [54.240.14.187])
by [non-Netflix link blocked] with ESMTPS id r6si57474qkb.470.2018.02.08.13.11.39
for
(version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128);
Thu, 08 Feb 2018 13:11:40 -0800 (PST)
Received-SPF: pass (google.com: domain of 010001617742739c-0daddaa6-5755-4638-ba13-3efbdad23cf8-000000@mailer.netflix.com designates 54.240.14.187 as permitted sender) client-ip=54.240.14.187;
Authentication-Results: mx.google.com;
dkim=pass header.i=@netflix.com header.s=wsh2ycm5iultkbdysii4ijqi22uurzvv header.b=Ju48Kj89;
dkim=pass [non-Netflix link blocked]header.s=224i4yxa5dv7c2xz3womw6peuasteono header.b=avCLQKcv;
spf=pass (google.com: domain of 010001617742739c-0daddaa6-5755-4638-ba13-3efbdad23cf8-000000@mailer.netflix.com designates 54.240.14.187 as permitted sender) smtp.mailfrom=010001617742739c-0daddaa6-5755-4638-ba13-3efbdad23cf8-000000@mailer.netflix.com;
dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=netflix.com
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple;
s=wsh2ycm5iultkbdysii4ijqi22uurzvv; d=netflix.com; t=1518124299;
i=@mailer.netflix.com;
h=Date:From:Sender:To:Message-ID:Subject:MIME-Version:Content-Type:List-Unsubscribe;
bh=rlAIU4r//XJoOnUVbGdtJyKyUSgzLS4h9fyRpIYRwDM=;
b=Ju48Kj89riXzwTCY5qLR2VOna/nF3YRgMbFfoTZBHuwohzdT4A0GnPqbKPCx7S8d
mVmLmB/SDY7JbBaOa77Zi4P6BF7m/J9a4viHA2J0jYKaI+P0S3pe15ArXAAU0uAEsux
l711QyDjDiEwiIDhKc4kFgf50L1DiXmtk/t/VyaI=
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple;
s=224i4yxa5dv7c2xz3womw6peuasteono; d=amazonses.com; t=1518124299;
h=Date:From:Sender:To:Message-ID:Subject:MIME-Version:Content-Type:List-Unsubscribe:Feedback-ID;
bh=rlAIU4r//XJoOnUVbGdtJyKyUSgzLS4h9fyRpIYRwDM=;
b=avCLQKcv9FVTs3xlGcfVJ4f88Nta3VGcn6ToUEDzfW9qpkcWpsAtD73vpIA8DoYZ
Dr8qab2YVZORQGiyVv9isM6U5mGDra0mgWxxBgWS/hZreVsUrCCTOS2uVdgS3Q+j8s0
0MjI648Nhsnr6tBhEcoNdDMK5hS2LO1bGCgSzwyE=
Date: Thu, 8 Feb 2018 21:11:39 +0000
From: Netflix
Sender: Netflix
To: <my email address>
Message-ID: <010001617742739c-0daddaa6-5755-4638-ba13-3efbdad23cf8-000000@email.amazonses.com>
Subject: Netflix password reset required
MIME-Version: 1.0
Content-Type: multipart/alternative;
boundary="----=_Part_132692_67010129.1518124299164"
X-To: <my email address>
Precedence: bulk
X-AppInfo: Netflix Mercury s.54d3d7d.2535
X-ProcEnc: BQAtAAEBEPvcm7MGMcMv7UifU3LjkkEQd/J3ASn5wkGSWcFnHH2ZTQ==
X-MailingID: mercury::LR::12699::50E29A9468B8D6EDBCE02367FACF7DCD64529BF0::<1617742739C@netflix.com>
X-GroupId: 4
X-localeCountry: en::AU
X-ProcInfo: TREADST

Netflix Jason

You can't possibly trust that.

Netflix Jason

You've already spoken to several Netflix experts.

You

yeah, I can actually, Gmail passed this

You

you have DKIM and SPF setup

You

that's definitely from you

Netflix Jason

Why would we ask you to reset the password?

You

how do I escalate this?

Netflix Jason

Forward it to phishing@netflix.com

You

I've been on this phone call for 35 minutes now

Netflix Jason

They were providing you the correct info.

Netflix Jason

Someone is trying to take over your account.

Netflix Jason

Trapping you to reset your password.

Netflix Jason

That is clearly not our process.

You

how do I get a technical person who knows what they are talking about?

Netflix Jason

You're chatting with one.

Netflix Jason

You've spoken to others as well.

You

so to be clear, you have an article that says that security emails will be sent out that passwords are reset

You

I've given you the headers, which show that it's come from your network

You

there is no way to spoof those headers

Netflix Jason

Those are notifications once you have reset the password.

You

do you know what DKIM actually is?

You

no, it's not!

Netflix Jason

Can you answer this question for me.

You

it's a notification that Netflix reset that password

You

sure

Netflix Jason

Have you tried to SIGN OUT then SIGN IN?

You

I reset the password!

You

from Netflix's site!

Netflix Jason

Did you get an error message that your password is wrong?

You

the old password no longer worked!

You

yes!

You

no, that the username and password WAS wrong

You

it had indeed been reset

Netflix Jason

Then it's correct someone took over your account.

Netflix Jason

You can see it.

You

and then they sent me an email that they had detected suspicious activity

Netflix Jason

You'll be able to see RECENT ACCOUNT ACCESS.

Netflix Jason

You'll see the IP ADDRESS, LOCATION, DEVICE. Date and TIME.

Netflix Jason

Did you check that out?

Netflix Jason

I see that you're already speaking to someone on the phone, do you still need me on this line?

You

they are literally contradicting you right now

Netflix Jason

Ok.

Netflix Jason

Anything else that I can do for you today? :)

You

someone will be having a word with you soon

Netflix Jason

That's alright.

Netflix Jason

By the way I sent you a link to our Help Center just for your reference. It's like our own version of Google where you can just type keywords of your inquiries or even error codes and it will show you exactly what needs to be done!

Netflix Jason

Oh, for future reference please check this link. Please
Click Here . Just a VERY important tip to save your time in the future, As long as you see that the servers are up on that link that means that the issue can be resolved by calling either your Internet service provider or you device manufacturer. I just want to make sure that you're fully equipped after this chat and I want to be able to still help out i:)

Netflix Jason

and if you want to check on updates to our NEW movie releases and feature updates please follow us on FACEBOOK or TWITTER. You can also chat with us real time on those sites. :)

Netflix Jason

It's been a pleasure chatting with you, have a good one! CHEERS! :)