content/browser/renderer_host/software_output_device.h - Issue 12542006: Implemented browser side software compositing.

Side by Side Diff: content/browser/renderer_host/software_output_device.h

Issue 12542006: Implemented browser side software compositing. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: Addressing review comments. Created 7 years, 9 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch | Annotate | Revision Log

« no previous file with comments | « content/browser/renderer_host/image_transport_factory.cc ('k') | content/browser/renderer_host/software_output_device_linux.cc » ('j') | content/browser/renderer_host/software_output_device_linux.cc » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 // Copyright (c) 2013 The Chromium Authors. All rights reserved.

	2 // Use of this source code is governed by a BSD-style license that can be

	3 // found in the LICENSE file.

	4

	5 #ifndef CONTENT_BROWSER_RENDERER_HOST_SOFTWARE_OUTPUT_DEVICE_H_

	6 #define CONTENT_BROWSER_RENDERER_HOST_SOFTWARE_OUTPUT_DEVICE_H_

	7

	8 #include "cc/software_output_device.h"

	9

	10 #if defined(OS_WIN)

	11 #include <windows.h>

	12 #elif defined(USE_X11)

	13 #include "ui/base/x/x11_util.h"

	14 #endif

	15

	16 namespace ui {

	17 class Compositor;

	18 }

	19

	20 namespace content {

	21

	22 class SoftwareOutputDevice : public cc::SoftwareOutputDevice {
	piman 2013/03/07 00:25:51 note: you have 2 completely separate implementatio note: you have 2 completely separate implementations of this class (a win one and a linux one). They don't share anything. It would be better to have 2 explicit classes (SoftwareOutputDeviceWin and SoftwareOutputDeviceLinux), and a single #ifdef in SoftwareTransportFactory::CreateOutputSurface slavi 2013/03/07 01:48:30 Done. Show quoted text On 2013/03/07 00:25:51, piman wrote: > note: you have 2 completely separate implementations of this class (a win one > and a linux one). They don't share anything. > > It would be better to have 2 explicit classes (SoftwareOutputDeviceWin and > SoftwareOutputDeviceLinux), and a single #ifdef in > SoftwareTransportFactory::CreateOutputSurface Done.
	23 public:

	24 explicit SoftwareOutputDevice(ui::Compositor* compositor);

	25

	26 virtual ~SoftwareOutputDevice();

	27

	28 virtual void Resize(const gfx::Size& viewport_size) OVERRIDE;

	29

	30 virtual void EndPaint(cc::SoftwareFrameData* frame_data) OVERRIDE;

	31

	32 private:

	33 ui::Compositor* compositor_;

	34

	35 #if defined(OS_WIN)

	36 HDC hdc_;

	37 BITMAPINFOHEADER bitmap_header_;
	piman 2013/03/07 00:25:51 Shouldn't this be a BITMAPINFO? The reinterpret_ca Shouldn't this be a BITMAPINFO? The reinterpret_cast in EndPaint is invalid (BITMAPINFO is a superset of BITMAPINFOHEADER). slavi 2013/03/07 01:48:30 That's fine since gfx::CreateBitmapHeader sets \|.b Show quoted text On 2013/03/07 00:25:51, piman wrote: > Shouldn't this be a BITMAPINFO? The reinterpret_cast in EndPaint is invalid > (BITMAPINFO is a superset of BITMAPINFOHEADER). That's fine since gfx::CreateBitmapHeader sets \|.biSize\| to sizeof(BITMAPINFOHEADER). It's rather a winapi quirk. BackingStoreWin does the same thing: https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re... Maybe change gfx::StretchDIBits to accept BITMAPINFOHEADER and have the reinterpret_cast inside instead of in users? slavi 2013/03/07 01:48:30 That's fine since gfx::CreateBitmapHeader sets \|.b Show quoted text On 2013/03/07 00:25:51, piman wrote: > Shouldn't this be a BITMAPINFO? The reinterpret_cast in EndPaint is invalid > (BITMAPINFO is a superset of BITMAPINFOHEADER). That's fine since gfx::CreateBitmapHeader sets \|.biSize\| to sizeof(BITMAPINFOHEADER). It's a winapi quirk. BackingStoreWin does the same thing: https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re... Maybe I should change gfx::StretchDIBits to accept BITMAPINFOHEADER and have the reinterpret_cast inside instead of in users? piman 2013/03/07 01:57:36 I'm no expert in the area - whatever the resident Show quoted text On 2013/03/07 01:48:30, slavi wrote: > On 2013/03/07 00:25:51, piman wrote: > > Shouldn't this be a BITMAPINFO? The reinterpret_cast in EndPaint is invalid > > (BITMAPINFO is a superset of BITMAPINFOHEADER). > > That's fine since gfx::CreateBitmapHeader sets \|.biSize\| to > sizeof(BITMAPINFOHEADER). It's a winapi quirk. > > BackingStoreWin does the same thing: > https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re... > > Maybe I should change gfx::StretchDIBits to accept BITMAPINFOHEADER and have the > reinterpret_cast inside instead of in users? I'm no expert in the area - whatever the resident expert wants. Still, you can make a BITMAPINFO, and then pass bitmap_header_.bmiHeader to gfx::StretchDIBits and not have the ugly reinterpret_cast.
	38 #elif defined(USE_X11)

	39 void ClearImage();

	40

	41 Display* display_;

	42 GC gc_;

	43 XImage* image_;

	44 #endif

	45 };

	46

	47 } // namespace content

	48

	49 #endif // CONTENT_BROWSER_RENDERER_HOST_SOFTWARE_OUTPUT_DEVICE_H_

OLD	NEW