Commit 0c8a66d4 authored by Tom Anderson's avatar Tom Anderson Committed by Commit Bot

[XProto] Restore colormap creation for transparent visuals

This fixes a regression I introduced after [1] which removed colormap
creation code.  It turns out the colormap is still necessary for
transparent visuals, so this CL adds it back.

[1] https://chromium.googlesource.com/chromium/src/+/ae0a09cefaca3445bf43b0abefe3d068794388c0

R=sky

Change-Id: I3d29ced609ab3f915f9b7cdbdce6f4e8b2a5dce1
Bug: 1106334,1107385
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2311070
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Auto-Submit: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790611}
parent d5d1bd5f
......@@ -26,8 +26,8 @@ void GetPlatformExtraDisplayAttribs(EGLenum platform_type,
// get it anyway.
if (platform_type != EGL_PLATFORM_ANGLE_TYPE_NULL_ANGLE) {
x11::VisualId visual_id;
ui::XVisualManager::GetInstance()->ChooseVisualForWindow(true, &visual_id,
nullptr, nullptr);
ui::XVisualManager::GetInstance()->ChooseVisualForWindow(
true, &visual_id, nullptr, nullptr, nullptr);
attributes->push_back(EGL_X11_VISUAL_ID_ANGLE);
attributes->push_back(static_cast<EGLAttrib>(visual_id));
}
......@@ -39,8 +39,8 @@ void ChoosePlatformCustomAlphaAndBufferSize(EGLint* alpha_size,
// can't use XVisualManager.
if (gl::GLSurfaceEGL::GetNativeDisplay() != EGL_DEFAULT_DISPLAY) {
uint8_t depth;
ui::XVisualManager::GetInstance()->ChooseVisualForWindow(true, nullptr,
&depth, nullptr);
ui::XVisualManager::GetInstance()->ChooseVisualForWindow(
true, nullptr, &depth, nullptr, nullptr);
*buffer_size = depth;
*alpha_size = *buffer_size == 32 ? 8 : 0;
}
......
......@@ -1150,7 +1150,7 @@ XVisualManager::XVisualManager() : connection_(x11::Connection::Get()) {
for (const auto& depth : connection_->default_screen().allowed_depths) {
for (const auto& visual : depth.visuals) {
visuals_[visual.visual_id] =
std::make_unique<XVisualData>(depth.depth, &visual);
std::make_unique<XVisualData>(connection_, depth.depth, &visual);
}
}
......@@ -1181,6 +1181,7 @@ XVisualManager::~XVisualManager() = default;
void XVisualManager::ChooseVisualForWindow(bool want_argb_visual,
x11::VisualId* visual_id,
uint8_t* depth,
x11::ColorMap* colormap,
bool* visual_has_alpha) {
base::AutoLock lock(lock_);
bool use_argb = want_argb_visual && IsCompositingManagerPresent() &&
......@@ -1191,15 +1192,16 @@ void XVisualManager::ChooseVisualForWindow(bool want_argb_visual,
if (visual_id)
*visual_id = visual;
bool success = GetVisualInfoImpl(visual, depth, visual_has_alpha);
bool success = GetVisualInfoImpl(visual, depth, colormap, visual_has_alpha);
DCHECK(success);
}
bool XVisualManager::GetVisualInfo(x11::VisualId visual_id,
uint8_t* depth,
x11::ColorMap* colormap,
bool* visual_has_alpha) {
base::AutoLock lock(lock_);
return GetVisualInfoImpl(visual_id, depth, visual_has_alpha);
return GetVisualInfoImpl(visual_id, depth, colormap, visual_has_alpha);
}
bool XVisualManager::OnGPUInfoChanged(bool software_rendering,
......@@ -1231,6 +1233,7 @@ bool XVisualManager::ArgbVisualAvailable() const {
bool XVisualManager::GetVisualInfoImpl(x11::VisualId visual_id,
uint8_t* depth,
x11::ColorMap* colormap,
bool* visual_has_alpha) {
auto it = visuals_.find(visual_id);
if (it == visuals_.end())
......@@ -1238,8 +1241,12 @@ bool XVisualManager::GetVisualInfoImpl(x11::VisualId visual_id,
XVisualData& data = *it->second;
const x11::VisualType& info = *data.info;
bool is_default_visual = visual_id == default_visual_id_;
if (depth)
*depth = data.depth;
if (colormap)
*colormap = is_default_visual ? x11::ColorMap{} : data.GetColormap();
if (visual_has_alpha) {
auto popcount = [](auto x) {
return std::bitset<8 * sizeof(decltype(x))>(x).count();
......@@ -1251,10 +1258,22 @@ bool XVisualManager::GetVisualInfoImpl(x11::VisualId visual_id,
return true;
}
XVisualManager::XVisualData::XVisualData(uint8_t depth,
XVisualManager::XVisualData::XVisualData(x11::Connection* connection,
uint8_t depth,
const x11::VisualType* info)
: depth(depth), info(info) {}
: depth(depth), info(info), connection_(connection) {}
// Do not free the colormap as this would uninstall the colormap even for
// non-Chromium clients.
XVisualManager::XVisualData::~XVisualData() = default;
x11::ColorMap XVisualManager::XVisualData::GetColormap() {
if (colormap_ == x11::ColorMap{}) {
colormap_ = connection_->GenerateId<x11::ColorMap>();
connection_->CreateColormap({x11::ColormapAlloc::None, colormap_,
connection_->default_root(), info->visual_id});
}
return colormap_;
}
} // namespace ui
......@@ -589,10 +589,12 @@ class COMPONENT_EXPORT(UI_BASE_X) XVisualManager {
void ChooseVisualForWindow(bool want_argb_visual,
x11::VisualId* visual_id,
uint8_t* depth,
x11::ColorMap* colormap,
bool* visual_has_alpha);
bool GetVisualInfo(x11::VisualId visual_id,
uint8_t* depth,
x11::ColorMap* colormap,
bool* visual_has_alpha);
// Called by GpuDataManagerImplPrivate when GPUInfo becomes available. It is
......@@ -613,17 +615,26 @@ class COMPONENT_EXPORT(UI_BASE_X) XVisualManager {
class XVisualData {
public:
XVisualData(uint8_t depth, const x11::VisualType* info);
XVisualData(x11::Connection* connection,
uint8_t depth,
const x11::VisualType* info);
~XVisualData();
uint8_t depth = 0;
const x11::VisualType* info = nullptr;
x11::ColorMap GetColormap();
const uint8_t depth;
const x11::VisualType* const info;
private:
x11::ColorMap colormap_{};
x11::Connection* const connection_;
};
XVisualManager();
bool GetVisualInfoImpl(x11::VisualId visual_id,
uint8_t* depth,
x11::ColorMap* colormap,
bool* visual_has_alpha);
mutable base::Lock lock_;
......
......@@ -258,11 +258,14 @@ void XWindow::Init(const Configuration& config) {
x11::VisualId visual_id = visual_id_;
uint8_t depth = 0;
x11::ColorMap colormap{};
XVisualManager* visual_manager = XVisualManager::GetInstance();
if (visual_id_ == x11::VisualId{} ||
!visual_manager->GetVisualInfo(visual_id_, &depth, &visual_has_alpha_)) {
visual_manager->ChooseVisualForWindow(
enable_transparent_visuals, &visual_id, &depth, &visual_has_alpha_);
!visual_manager->GetVisualInfo(visual_id_, &depth, &colormap,
&visual_has_alpha_)) {
visual_manager->ChooseVisualForWindow(enable_transparent_visuals,
&visual_id, &depth, &colormap,
&visual_has_alpha_);
}
// x.org will BadMatch if we don't set a border when the depth isn't the
......@@ -278,6 +281,7 @@ void XWindow::Init(const Configuration& config) {
req.depth = depth;
req.c_class = x11::WindowClass::InputOutput;
req.visual = visual_id;
req.colormap = colormap;
xwindow_ = connection_->GenerateId<x11::Window>();
req.wid = xwindow_;
connection_->CreateWindow(req);
......
......@@ -50,7 +50,7 @@ bool InitializeVisuals() {
std::unique_ptr<base::Environment> env(base::Environment::Create());
has_compositing_manager = env->HasVar("_CHROMIUM_INSIDE_XVFB");
ui::XVisualManager::GetInstance()->ChooseVisualForWindow(
has_compositing_manager, nullptr, &depth, &using_argb_visual);
has_compositing_manager, nullptr, &depth, nullptr, &using_argb_visual);
if (using_argb_visual)
EXPECT_EQ(32, depth);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment