Skip to content

Commit

Permalink
OpenGL: Ensure that macOS uses the same (sRGB) colour space everywhere
Browse files Browse the repository at this point in the history
  • Loading branch information
@reuk
reuk committed Sep 27, 2022
1 parent 2ae87f9 commit 19175ff
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 20 deletions.
6 changes: 6 additions & 0 deletions modules/juce_graphics/native/juce_mac_CoreGraphicsContext.h
View file
Expand Up @@ -53,12 +53,18 @@ namespace detail
void operator() (CGGradientRef ptr) const noexcept { CGGradientRelease (ptr); }
};

struct ColorDelete
{
void operator() (CGColorRef ptr) const noexcept { CGColorRelease (ptr); }
};

//==============================================================================
using ColorSpacePtr = std::unique_ptr<CGColorSpace, ColorSpaceDelete>;
using ContextPtr = std::unique_ptr<CGContext, ContextDelete>;
using DataProviderPtr = std::unique_ptr<CGDataProvider, DataProviderDelete>;
using ImagePtr = std::unique_ptr<CGImage, ImageDelete>;
using GradientPtr = std::unique_ptr<CGGradient, GradientDelete>;
using ColorPtr = std::unique_ptr<CGColor, ColorDelete>;
}

//==============================================================================
Expand Down
9 changes: 7 additions & 2 deletions modules/juce_graphics/native/juce_mac_CoreGraphicsContext.mm
View file
Expand Up @@ -404,8 +404,13 @@ static CGBitmapInfo getCGImageFlags (const Image::PixelFormat& format)

if (fillType.isColour())
{
CGContextSetRGBFillColor (context.get(), fillType.colour.getFloatRed(), fillType.colour.getFloatGreen(),
fillType.colour.getFloatBlue(), fillType.colour.getFloatAlpha());
const CGFloat components[] { fillType.colour.getFloatRed(),
fillType.colour.getFloatGreen(),
fillType.colour.getFloatBlue(),
fillType.colour.getFloatAlpha() };

const detail::ColorPtr color { CGColorCreate (rgbColourSpace.get(), components) };
CGContextSetFillColorWithColor (context.get(), color.get());
CGContextSetAlpha (context.get(), 1.0f);
}
}
Expand Down
9 changes: 5 additions & 4 deletions modules/juce_gui_basics/native/juce_mac_NSViewComponentPeer.mm
View file
Expand Up @@ -178,6 +178,7 @@ static constexpr int translateVirtualToAsciiKeyCode (int keyCode) noexcept
styleMask: getNSWindowStyleMask (windowStyleFlags)
backing: NSBackingStoreBuffered
defer: YES];
[window setColorSpace: [NSColorSpace sRGBColorSpace]];
setOwner (window, this);

if (@available (macOS 10.10, *))
Expand Down Expand Up @@ -1617,16 +1618,16 @@ void setHasChangedSinceSaved (bool b) override
*/
PerScreenDisplayLinks::Connection connection
{
sharedDisplayLinks->registerFactory ([this] (auto* screen)
sharedDisplayLinks->registerFactory ([this] (CGDirectDisplayID display)
{
return [peerRef = WeakReference<NSViewComponentPeer> { this }, screen]
return [peerRef = WeakReference<NSViewComponentPeer> { this }, display]
{
MessageManager::callAsync ([peerRef, screen]
MessageManager::callAsync ([peerRef, display]
{
if (auto* peer = peerRef.get())
if (auto* peerView = peer->view)
if (auto* peerWindow = [peerView window])
if (screen == [peerWindow screen])
if (display == ScopedDisplayLink::getDisplayIdForScreen ([peerWindow screen]))
peer->setNeedsDisplayRectangles();
});
};
Expand Down
27 changes: 16 additions & 11 deletions modules/juce_gui_basics/native/juce_mac_PerScreenDisplayLinks.h
View file
Expand Up @@ -97,9 +97,14 @@ class FunctionNotificationCenterObserver
class ScopedDisplayLink
{
public:
static CGDirectDisplayID getDisplayIdForScreen (NSScreen* screen)
{
return (CGDirectDisplayID) [[screen.deviceDescription objectForKey: @"NSScreenNumber"] unsignedIntegerValue];
}

ScopedDisplayLink (NSScreen* screenIn, std::function<void()> onCallbackIn)
: screen (screenIn),
link ([display = (CGDirectDisplayID) [[screen.deviceDescription objectForKey: @"NSScreenNumber"] unsignedIntegerValue]]
: displayId (getDisplayIdForScreen (screenIn)),
link ([display = displayId]
{
CVDisplayLinkRef ptr = nullptr;
const auto result = CVDisplayLinkCreateWithCGDisplay (display, &ptr);
Expand Down Expand Up @@ -133,7 +138,7 @@ class ScopedDisplayLink
CVDisplayLinkStop (link.get());
}

NSScreen* getScreen() const { return screen; }
CGDirectDisplayID getDisplayId() const { return displayId; }

double getNominalVideoRefreshPeriodS() const
{
Expand All @@ -155,7 +160,7 @@ class ScopedDisplayLink
}
};

NSScreen* screen = nullptr;
CGDirectDisplayID displayId;
std::unique_ptr<std::remove_pointer_t<CVDisplayLinkRef>, DisplayLinkDestructor> link;
std::function<void()> onCallback;

Expand All @@ -179,7 +184,7 @@ class PerScreenDisplayLinks
}

using RefreshCallback = std::function<void()>;
using Factory = std::function<RefreshCallback (NSScreen*)>;
using Factory = std::function<RefreshCallback (CGDirectDisplayID)>;

/*
Automatically unregisters a CVDisplayLink callback factory when ~Connection() is called.
Expand Down Expand Up @@ -235,12 +240,12 @@ class PerScreenDisplayLinks
return { *this, factories.begin() };
}

double getNominalVideoRefreshPeriodSForScreen (NSScreen* screen) const
double getNominalVideoRefreshPeriodSForScreen (CGDirectDisplayID display) const
{
const ScopedLock lock (mutex);

for (const auto& link : links)
if (link.getScreen() == screen)
if (link.getDisplayId() == display)
return link.getNominalVideoRefreshPeriodS();

return 0.0;
Expand All @@ -265,7 +270,7 @@ class PerScreenDisplayLinks
std::vector<RefreshCallback> callbacks;

for (auto& factory : factories)
callbacks.push_back (factory (screen));
callbacks.push_back (factory (ScopedDisplayLink::getDisplayIdForScreen (screen)));

// This is the callback that will actually fire in response to this screen's display
// link callback.
Expand All @@ -291,9 +296,9 @@ class PerScreenDisplayLinks
// This is a list rather than a vector because ScopedDisplayLink is non-moveable.
std::list<ScopedDisplayLink> links;

FunctionNotificationCenterObserver observer { NSApplicationDidChangeScreenParametersNotification,
nullptr,
[this] { refreshScreens(); } };
FunctionNotificationCenterObserver screenParamsObserver { NSApplicationDidChangeScreenParametersNotification,
nullptr,
[this] { refreshScreens(); } };
};

} // namespace juce
1 change: 1 addition & 0 deletions modules/juce_opengl/native/juce_OpenGL_osx.h
View file
Expand Up @@ -123,6 +123,7 @@ class OpenGLContext::NativeContext
void shutdownOnRenderThread() { deactivateCurrentContext(); }

bool createdOk() const noexcept { return getRawContext() != nullptr; }
NSOpenGLView* getNSView() const noexcept { return view; }
NSOpenGLContext* getRawContext() const noexcept { return renderContext; }
GLuint getFrameBufferID() const noexcept { return 0; }

Expand Down
18 changes: 15 additions & 3 deletions modules/juce_opengl/opengl/juce_OpenGLContext.cpp
View file
Expand Up @@ -980,15 +980,27 @@ class OpenGLContext::CachedImage : public CachedComponentImage
void updateScreen()
{
const auto screen = getCurrentScreen();
lastScreen = screen;
const auto display = ScopedDisplayLink::getDisplayIdForScreen (screen);

const auto newRefreshPeriod = sharedDisplayLinks->getNominalVideoRefreshPeriodSForScreen (screen);
if (lastDisplay.exchange (display) == display)
return;

const auto newRefreshPeriod = sharedDisplayLinks->getNominalVideoRefreshPeriodSForScreen (display);

if (newRefreshPeriod != 0.0 && std::exchange (refreshPeriod, newRefreshPeriod) != newRefreshPeriod)
nativeContext->setNominalVideoRefreshPeriodS (newRefreshPeriod);

updateColourSpace();
}

void updateColourSpace()
{
if (auto* view = nativeContext->getNSView())
if (auto* window = [view window])
[window setColorSpace: [NSColorSpace sRGBColorSpace]];
}

std::atomic<NSScreen*> lastScreen { nullptr };
std::atomic<CGDirectDisplayID> lastDisplay { 0 };
double refreshPeriod = 0.0;

FunctionNotificationCenterObserver observer { NSWindowDidChangeScreenNotification,
Expand Down

2 comments on commit 19175ff

@yairchu
Copy link

@yairchu yairchu commented on 19175ff Sep 28, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that colo[u]rs in the OpenGL renderer should now match those of the CoreGraphics renderer?

@reuk
Copy link
Member Author

@reuk reuk commented on 19175ff Sep 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, hopefully.

Please sign in to comment.