system_server can potentially monitor uevent devices that are
of no use to it. For instance, an accelerometer implementation
as uevents. This would cause the process to be busy when
unnecessary. If a device cannot be classified, don't monitor it.
Change-Id: Ib2c93105e7d746d9c1a7414bea8bab3fb8c0b70a
On single-touch devices, pointer up/down is signalled by a BTN_TOUCH
key event. Previously we handled BTN_TOUCH immediately but some drivers
may produce the sequence BTN_TOUCH, ABS_X, ABS_Y, SYN_REPORT on pointer down
which caused us to emit a bad initial pointer down location.
Now we wait for SYN_REPORT before reporting the up or down.
On multi-touch devices, pointer up can be signalled by as little as
the sequence SYN_MT_REPORT, SYN_REPORT. This change ensures that we
handle this case.
Added support for reading ABS_MT_PRESSURE when available.
Corrected mapping of touchMajor/touchMinor on single touch devices.
Minor code cleanup.
Change-Id: Ic7ec4811241ed85a06e59b8a839ca05180d491d4
Sometimes the wrong fd was accessed when the device was addressed
by device id.
The earlier implementation assumed that two arrays were in sync
but one of them was compacted when devices were removed. Instead
of that dependency the device now keeps track of it's file descriptor.
Change-Id: Ib0f320603aafb07ded354bc3687de9759c9068f2
Previously, the input dispatcher assumed that the input channel's
receive pipe file descriptor was a sufficiently unique identifier for
looking up input channels in its various tables. However, it can happen
that an input channel is disposed and then a new input channel is
immediately created that reuses the same file descriptor. Ordinarily
this is not a problem, however there is a small opportunity for a race
to arise in InputQueue.
When InputQueue receives an input event from the dispatcher, it
generates a finishedToken that encodes the channel's receive pipe fd,
and a sequence number. The finishedToken is used by the ViewRoot
as a handle for the event so that it can tell the InputQueue when
the event has finished being processed.
Here is the race:
1. InputQueue receives an input event, assigns a new finishedToken.
2. ViewRoot begins processing the input event.
3. During processing, ViewRoot unregisters the InputChannel.
4. A new InputChannel is created and is registered with the Input Queue.
This InputChannel happens to have the same receive pipe fd as
the one previously registered.
5. ViewRoot tells the InputQueue that it has finished processing the
input event, passing along the original finishedToken.
6. InputQueue throws an exception because the finishedToken's receive
pipe fd is registered but the sequence number is incorrect so it
assumes that the client has called finish spuriously.
The fix is to include a unique connection id within the finishedToken so
that the InputQueue can accurately confirm that the token belongs to
the currently registered InputChannel rather than to an old one that
happened to have the same receive pipe fd. When it notices this, it
ignores the spurious finish.
I've also made a couple of other small changes to avoid similar races
elsewhere.
This patch set also includes a fix to synthesize a finished signal
when the input channel is unregistered on the client side to
help keep the server and client in sync.
Bug: 2834068
Change-Id: I1de34a36249ab74c359c2c67a57e333543400f7b
Also fixed bug where old touch screen size could be reported by
getMotionRange if an orientation change occurred but the user has not
yet touched the screen.
Bug: 2877345
Change-Id: I7878f47458f310ed6ebe6a5d1b2c9bec2c598ab9
Added a new asynchronous injection mode and made the existing
synchronization mechanism more robust.
Change-Id: Ia4aa04fd9b75ea2461a844c5b7933c831c1027e6
Refactored the input reader so that each raw input protocol is handled
by a separate subclass of the new InputMapper type. This way, behaviors
pertaining to keyboard, trackballs, touchscreens, switches and other
devices are clearly distinguished for improved maintainability.
Added partial support for describing capabilities of input devices
(incomplete and untested for now, will be fleshed out in later commits).
Simplified EventHub interface somewhat since InputReader is taking over
more of the work.
Cleaned up some of the interactions between InputManager and
WindowManagerService related to reading input state.
Fixed swiping finger from screen edge into display area.
Added logging of device information to 'dumpsys window'.
Change-Id: I17faffc33e3aec3a0f33f0b37e81a70609378612
This significantly re-works the native key dispatching code to
allow events to be pre-dispatched to the current IME before
being processed by native code. It introduces one new public
API, which must be called after retrieving an event if the app
wishes for it to be pre-dispatched.
Currently the native code will only do pre-dispatching of
system keys, to avoid significant overhead for gaming input.
This should be improved to be smarted, filtering for only
keys that the IME is interested in. Unfortunately IMEs don't
currently provide this information. :p
Change-Id: Ic1c7aeec8b348164957f2cd88119eb5bd85c2a9f
Added several new coordinate values to MotionEvents to capture
touch major/minor area, tool major/minor area and orientation.
Renamed NDK input constants per convention.
Added InputDevice class in Java which will eventually provide
useful information about available input devices.
Added APIs for manufacturing new MotionEvent objects with multiple
pointers and all necessary coordinate data.
Fixed a bug in the input dispatcher where it could get stuck with
a pointer down forever.
Fixed a bug in the WindowManager where the input window list could
end up containing stale removed windows.
Fixed a bug in the WindowManager where the input channel was being
removed only after the final animation transition had taken place
which caused spurious WINDOW DIED log messages to be printed.
Change-Id: Ie55084da319b20aad29b28a0499b8dd98bb5da68
And also:
- APIs to show and hide the IME, and control its interaction with the app.
- APIs to tell the app when its window resizes and needs to be redrawn.
- API to tell the app the content rectangle of its window (to layout
around the IME or status bar).
There is still a problem with IME interaction -- we need a way for the
app to deliver events to the IME before it handles them, so that for
example the back key will close the IME instead of finishing the app.
Change-Id: I37b75fc2ec533750ef36ca3aedd2f0cc0b5813cd
Also other cleanup and fixes:
- We now properly set the default window format to 565.
- New APIs to set the window format and flags from native code.
- Tweaked glue for simpler handling of the "destroy" message.
- Um, other stuff.
Change-Id: Id7790a21a2fa9a19b91854d225324a7c1e7c6ade
This factors out the boiler-plate code from the sample
app to a common glue code that can be used for everyone
writing this style of app: a dedicated app thread that
takes care of waiting for events and processing them.
As part of doing this, ALooper has a new facility to allow
registration of fds that cause ALooper_pollOnce() to return
the fd that has data, allowing the app to drive the loop
without callbacks. Hopefully this makes some people feel better. :)
Also do some other cleanup of the ALooper API, plus some
actual documentation.
Change-Id: Ic53bd56bdf627e3ba28a3c093faa06a92be522b8
Removed old input dispatch code.
Refactored the policy callbacks.
Pushed a tiny bit of the power manager state down to native.
Fixed long press on MENU.
Made the virtual key detection and cancelation a bit more precise.
Change-Id: I5d8c1062f7ea0ab3b54c6fadb058c4d5f5a9e02e
On omap3 h/w we force opaque formats to RGB_565 instead of RGBX_8888
because the GL driver doesn't support it. RGBX_8888 is always remapped
to RGBA_8888.
Change-Id: I0bfabeb98c8d3a399079e6797cf2a0ee95915324
Not yet hooked up to anything in the NDK, but requires renaming
the existing android_native_window_t type everywhere.
Change-Id: Iffee6ea39c93b8b34e20fb69e4d2c7c837e5ea2e
This change mainly unwinds a premature optimization in the
dispatch pipeline.
To test HOME injection, run 'adb shell input keyevent 3'.
Change-Id: I1c4b7377c205da7c898014b8b07fc6dc1d46e4dd
The native code now maintains a list of all keys that may use
default handling. If the app finishes one of these keys
without handling it, the key will be passed back off to Java
for default treatment.
Change-Id: I6a842a0d728eeafa4de7142fae573f8c11099e18
Set a default orientation of ROTATION_0.
Added some more careful checks based on whether we have valid
absolute axis information from the driver.
Reset key repeating during configuration changes since the keyboard
device may have been removed.
Change-Id: I685960828acffcb17595fc5683309e8064a76714
Target identification is now fully native.
Fixed a couple of minor issues related to input injection.
Native input enabled by default, can be disabled by setting
WindowManagerPolicy.ENABLE_NATIVE_INPUT_DISPATCH to false.
Change-Id: I7edf66ed3e987cc9306ad4743ac57a116af452ff
Sometimes the wrong fd was accessed when the device was addressed
by device id.
The earlier implementation assumed that two arrays were in sync
but one of them was compacted when devices were removed. Instead
of that dependency the device now keeps track of it's file descriptor.
Change-Id: I2b8a793d76b89ab464ae830482b309fe86031671
Provides the basic infrastructure for a
NativeActivity's native code to get an object representing
its event stream that can be used to read input events.
Still work to do, probably some API changes, and reasonable
default key handling (so that for example back will still
work).
Change-Id: I6db891bc35dc9683181d7708eaed552b955a077e
Added ANRs handling.
Added event injection.
Fixed a NPE ActivityManagerServer writing ANRs to the drop box.
Fixed HOME key interception.
Fixed trackball reporting.
Fixed pointer rotation in landscape mode.
Change-Id: I50340f559f22899ab924e220a78119ffc79469b7
Added more tests.
Fixed a regression in Vector.
Fixed bugs in pointer tracking.
Fixed a starvation issue in PollLoop when setting or removing callbacks.
Fixed a couple of policy nits.
Modified the internal representation of MotionEvent to be more
efficient and more consistent.
Added code to skip/cancel virtual key processing when there are multiple
pointers down. This helps to better disambiguate virtual key presses
from stray touches (such as cheek presses).
Change-Id: I2a7d2cce0195afb9125b23378baa94fd2fc6671c
Refactored the code to eliminate potential deadlocks due to re-entrant
calls from the policy into the dispatcher. Also added some plumbing
that will be used to notify the framework about ANRs.
Change-Id: Iba7a10de0cb3c56cd7520d6ce716db52fdcc94ff
The old dispatch mechanism has been left in place and continues to
be used by default for now. To enable native input dispatch,
edit the ENABLE_NATIVE_DISPATCH constant in WindowManagerPolicy.
Includes part of the new input event NDK API. Some details TBD.
To wire up input dispatch, as the ViewRoot adds a window to the
window session it receives an InputChannel object as an output
argument. The InputChannel encapsulates the file descriptors for a
shared memory region and two pipe end-points. The ViewRoot then
provides the InputChannel to the InputQueue. Behind the
scenes, InputQueue simply attaches handlers to the native PollLoop object
that underlies the MessageQueue. This way MessageQueue doesn't need
to know anything about input dispatch per-se, it just exposes (in native
code) a PollLoop that other components can use to monitor file descriptor
state changes.
There can be zero or more targets for any given input event. Each
input target is specified by its input channel and some parameters
including flags, an X/Y coordinate offset, and the dispatch timeout.
An input target can request either synchronous dispatch (for foreground apps)
or asynchronous dispatch (fire-and-forget for wallpapers and "outside"
targets). Currently, finding the appropriate input targets for an event
requires a call back into the WindowManagerServer from native code.
In the future this will be refactored to avoid most of these callbacks
except as required to handle pending focus transitions.
End-to-end event dispatch mostly works!
To do: event injection, rate limiting, ANRs, testing, optimization, etc.
Change-Id: I8c36b2b9e0a2d27392040ecda0f51b636456de25
Surfaces can now be parcelized and sent to remote
processes. When a surface crosses a process
boundary, it looses its connection with the
current process and gets attached to the new one.
Change-Id: I39c7b055bcd3ea1162ef2718d3d4b866bf7c81c0
opaque 32-bits windows are now allocated as RGBX_8888 buffers and
SurfaceFlinger always uses GL_MODULATE instead of trying to
optimize to GL_REPLACE when possible (makes no sense on
h/w accelerated GL).
we still have a small hack for devices that don't support
RGBX_8888 in their gralloc implementation where we revert to
RGBA_8888.
the framebuffer implementation doesn't do anything special with this
but the surfaceflinger implementation makes sure the surface is not used
by two APIs simultaneously.
Change-Id: Id4ca8ef7093d68846abc2ac814327cc40a64b66b
We've gotten lucky to date: the previous calculation of bitmask array
sizes, (maxval+1)/8 only works properly when 'maxval' is one less than
a multiple of 8. Fortunately, this has either been the case for us,
or there has been sufficient 'unused' space at the end of the defined
max value range that we haven't wound up overreading/overwriting the
allocated buffers.
Change-Id: I563a93a86644ab9f19489565e06c28e06bb53abc
We now only consider a device to be a default keyboard if its name
has "-keypad". A hack, but whatever.
Also add some debug logging for the input state to help identify such
issues in the future.
Add a Flattenable interface to libutils which can be used to flatten
an object into bytestream + filedescriptor stream.
Parcel is modified to handle Flattenable. And GraphicBuffer implements
Flattenable.
Except for the overlay classes libui is now independent of libbinder.
Merge commit '425324e97bba75cd69bb6c81de6248529540e6fe'
* commit '425324e97bba75cd69bb6c81de6248529540e6fe':
Fix failure to open AVRCP input device due to EPERM.
Sleep for 100us and try to open the input device again if it fails, with a
maximum of 10 attempts.
We need the retry logic because setting permissions on a new input device is
racy. The init process watches for new input device (via uevent) and sets the
permission on them in devices.c:make_device(). However at the same time
EventHub.cpp watches for new input devices from the system_server process, and
immediately tries to open them. I can't see a simple way to avoid this race
condition.
As best as I can tell this race condition has always exisited.
There must have been some timing change that happened recently that causes us
to hit this race condition much more often. See repro notes in referenced bug.
Bug: 2375632
Surface::validate() could sometimes dereference a null pointer before checking it wasn't null.
This will prevent the application to crash when given bad parameters or used incorrectly.
However, the bug above probably has another cause.
we lost the concept of vertical stride when moving video playback to EGLImage.
Here we bring it back in a somewhat hacky-way that will work only for the
softgl/mdp backend.
Use EGLImageKHR instead of copybit directly.
We now have the basis to use streaming YUV textures (well, in fact
we already are). When/if we use the GPU instead of the MDP we'll
need to make sure it supports the appropriate YUV format.
Also make sure we compile if EGL_ANDROID_image_native_buffer is not supported
when running out of memory, a null handle is returned but the error code may not be set.
In that case we need to return NO_MEMORY instead of NO_ERROR, so that the calling code
won't try to dereference the null pointer.
This also fixes [2152536] ANR in browser
When SF is enqueuing buffers faster than SF dequeues them.
The update flag in SF is not counted and under some situations SF will only
dequeue the first buffer. The state at this point is not technically
corrupted, it's valid, but just delayed by one buffer.
In the case of the Browser ANR, because the last enqueued buffer was delayed
the resizing of the current buffer couldn't happen.
The system would always fall back onto its feet if anything -else- in
tried to draw, because the "late" buffer would be picked up then.
A window is created and the browser is about to render into it the
very first time, at that point it does an IPC to SF to request a new
buffer. Meanwhile, the window manager removes that window from the
list and the shared memory block it uses is marked as invalid.
However, at that point, another window is created and is given the
same index (that just go freed), but a different identity and resets
the "invalid" bit in the shared block. When we go back to the buffer
allocation code, we're stuck because the surface we're allocating for
is gone and we don't detect it's invalid because the invalid bit has
been reset.
It is not sufficient to check for the invalid bit, I should
also check that identities match.
When EGLImage extension is not available, SurfaceFlinger will fallback to using
glTexImage2D and glTexSubImage2D instead, which requires 50% more memory and an
extra copy. However this code path has never been exercised and had some bugs
which this patch fix.
Mainly the scale factor wasn't computed right when falling back on glDrawElements.
We also fallback to this mode of operation if a buffer doesn't have the adequate
usage bits for EGLImage usage.
This changes only code that is currently not executed. Some refactoring was needed to
keep the change clean. This doesn't change anything functionaly.
The ANR is caused by SurfaceFlinger waiting for buffers of a removed surface to become availlable.
When it is removed from the current list, a Surface is marked as NO_INIT, which causes SF to return
immediately in the above case. For some reason, the surface here wasn't marked as NO_INIT.
This change makes the code more robust by always (irregadless or errors) setting the NO_INIT status
in all code paths where a surface is removed from the list.
Additionaly added more information in the logs, should this happen again.
a new method, compostionComplete() is added to the framebuffer hal, it is used by surfaceflinger to signal the driver that the composition is complete, BEFORE it releases its client. This gives a chance to the driver to
Take 2. We needed to check that the usage flags are "good enough" as opposed to "the same".
This reverts commit 8f17a762fe9e9f31e4e86cb60ff2bfb6b10fdee6.
Appears to have been broken by:
commit 9779b221e999583ff89e0dfc40e56398737adbb3
Author: Mathias Agopian <mathias@google.com>
Date: Mon Sep 7 16:32:45 2009 -0700
fix [2068105] implement queueBuffer/lockBuffer/dequeueBuffer properly
For some reason we don't like to have "-lpthread" globally -- it's a no-op
on device builds, but required for many host tools and all sim binaries --
so adding the use of pthread calls requires adding the library explicitly.
Rewrote SurfaceFlinger's buffer management from the ground-up.
The design now support an arbitrary number of buffers per surface, however the current implementation is limited to four. Currently only 2 buffers are used in practice.
The main new feature is to be able to dequeue all buffers at once (very important when there are only two).
A client can dequeue all buffers until there are none available, it can lock all buffers except the last one that is used for composition. The client will block then, until a new buffer is enqueued.
The current implementation requires that buffers are locked in the same order they are dequeued and enqueued in the same order they are locked. Only one buffer can be locked at a time.
eg. Allowed sequence: DQ, DQ, LOCK, Q, LOCK, Q
eg. Forbidden sequence: DQ, DQ, LOCK, LOCK, Q, Q
This addresses a few parts of the bug:
- There was a small issue in the window manager where we could show a window
too early before the transition animation starts, which was introduced
by the recent wallpaper work. This was the cause of the flicker when
starting the dialer for the first time.
- There was a much larger problem that has existing forever where moving
an application token to the front or back was not synchronized with the
application animation transaction. This was the cause of the flicker
when hanging up (now that the in-call screen moves to the back instead
of closing and we always have a wallpaper visible). The approach to
solving this is to have the window manager go ahead and move the app
tokens (it must in order to keep in sync with the activity manager), but
to delay the actual window movement: perform the movement to front when
the animation starts, and to back when it ends. Actually, when the
animation ends, we just go and completely rebuild the window list to
ensure it is correct, because there can be ways people can add windows
while in this intermediate state where they could end up at the wrong
place once we do the delayed movement to the front or back. And it is
simply reasuring to know that every time we finish a full app transition,
we re-evaluate the world and put everything in its proper place.
Also included in this change are a few little tweaks to the input system,
to perform better logging, and completely ignore input devices that do not
have any of our input classes. There is also a little cleanup of evaluating
configuration changes to not do more work than needed when an input
devices appears or disappears, and to only log a config change message when
the config is truly changing.
Change-Id: Ifb2db77f8867435121722a6abeb946ec7c3ea9d3
what happened is that the efective pixel format is calculated by SF but Surface nevew had access to it directly.
in particular this caused query(FORMAT) to return the requested format instead of the effective format.
This change makes SurfaceHolder.setType(GPU) obsolete (it's now ignored).
Added an API to android_native_window_t to allow extending the functionality without ever breaking binary compatibility. This is used to implement the new set_usage() API. This API needs to be called by software renderers because the default is to use usage flags suitable for h/w.
* changes:
update most gl tests to use EGLUtils
added two EGL helpers for selecting a config matching a certain pixelformat or native window type
added NATIVE_WINDOW_FORMAT attribute to android_native_window_t
The major things going on here:
- The MotionEvent API is now extended to included "pointer ID" information, for
applications to keep track of individual fingers as they move up and down.
PointerLocation has been updated to take advantage of this.
- The input system now has logic to generate MotionEvents with the new ID
information, synthesizing an identifier as new points are down and trying to
keep pointer ids consistent across events by looking at the distance between
the last and next set of pointers.
- We now support the new multitouch driver protocol, and will use that instead
of the old one if it is available. We do NOT use any finger id information
coming from the driver, but always synthesize pointer ids in user space.
(This is simply because we don't yet have a driver reporting this information
from which to base an implementation on.)
- Increase maximum number of fingers to 10. This code has only been used
with a driver that reports up to 2, so no idea how more will actually work.
- Oh and the input system can now detect and report physical DPAD devices.
there was several issues:
- when a surface was made non-current, the last frame wasn't shown and the buffer could stay locked
- when a surface was made current the 2nd time, it would not dequeue a new buffer
now, queue/dequeue are done when the surface is made current.
for this to work, a new query() hook had to be added on android_native_window_t, it allows to retrieve some attributes of a window (currently only width and height).
This will be used to avoid unnecessarily listening to data from sensors
that function as event devices.
Signed-off-by: Mike Lockwood <lockwood@android.com>
The kernel can now publish a property describing the layout of virtual
hardware buttons on the touchscreen. These outside of the display
area (outside of the absolute x and y controller range the driver
reports), and when the user presses on them a key event will be
generated rather than a touch event.
This also includes a number of tweaks to the absolute controller
processing to make things work better on the new screens. For
example, we now reject down events outside of the display area.
Still left to be done is the ability to cancel a key down event,
so the user can slide up from the virtual keys to the touch screen
without causing a virtual key to execute.
Merge commit 'c7396025e59524e7ef639fd86fc23123939ee91c'
* commit 'c7396025e59524e7ef639fd86fc23123939ee91c':
Return CAMERA_ERROR_SERVER_DIED to camera app when camera service dies (bug 1956726)
Merge commit 'c44989d6c7bcc761fb37f54fd37aac2070ba8e5e'
* commit 'c44989d6c7bcc761fb37f54fd37aac2070ba8e5e':
move ui/Time.cpp to core/jni, since this is the only place it is used
Merge commit '3d7b8d1aa6a362292f56defbe8fb2d5653f79282'
* commit '3d7b8d1aa6a362292f56defbe8fb2d5653f79282':
Use a ref-counted callback interface for Camera.
This allows the camera service to hang onto the callback interface
until all callbacks have been processed. This prevents problems
where pending callbacks in binder worker threads are processed
after the Java camera object and its associated native resources
have been released.
Bug 1884362
- return "const" objects for overloaded operators to disallow constructs like: (a+b) = c;
- don't return references to non-static members, it's not always safe.
- Point.cpp was empty, so get rid of it
- make sure that all binder Bn classes define a ctor and dtor in their respective library.
This avoids duplication of the ctor/dtor in libraries where these objects are instantiated.
This is also cleaner, should we want these ctor/dtor to do something one day.
- same change as above for some Bp classes and various other non-binder classes
- moved the definition of CHECK_INTERFACE() in IInterface.h instead of having it everywhere.
- improved the CHECK_INTERFACE() macro so it calls a single method in Parcel, instead of inlining its code everywhere
- IBinder::getInterfaceDescriptor() now returns a "const String16&" instead of String16, which saves calls to String16 and ~String16
- implemented a cache for BpBinder::getInterfaceDescriptor(), since this does an IPC. HOWEVER, this method never seems to be called.
The cache makes BpBinder bigger, so we need to figure out if we need this method at all.
This is the second half of bug 1837832. Modifies the camera client
and camera service to use the new binder interface. Removes the
old binder interface. There will be one more part to this change
to surface the undefined callbacks to the Java layer so that
partners can implement new features without having to touch the
stack.
This is the first step in a multi-step change to move from the old
specific callbacks to a generic callback. This will allow future
flexibility in the interface without requiring binder rewrites.
Bug 1837832
- Currently the lock/unlock path is naive and is done for each drawing operation (glDrawElements and glDrawArrays). this should be improved eventually.
- factor all the lock/unlock code in SurfaceBuffer.
- fixed "showupdate" so it works even when we don't have preserving eglSwapBuffers().
- improved the situation with the dirty-region and fixed a problem that caused GL apps to not update.
- make use of LightRefBase() where needed, instead of duplicating its implementation
- add LightRefBase::getStrongCount()
- renamed EGLNativeWindowSurface.cpp to FramebufferNativeWindow.cpp
- disabled copybits test, since it clashes with the new gralloc api
- Camera/Video will be fixed later when we rework the overlay apis
Add a factory method that creates a Camera object from a remote client
Next:
The changes in authordriver.cpp and android_camera_input.cpp will come.
and the constructor for Camera object will be removed.
The WindowManager side of Surface.java holds a SurfaceControl, while the client-side holds a Surface. When the client is in the system process, Surface.java holds both (which is a problem we'll try to fix later).
SurfaceControl is used for controling the geometry of the surface (for the WM), while Surface is used to access the buffers (for SF's clients).
SurfaceFlingerClient now uses the SurfaceID instead of Surface*.
Currently Surface still has the SurfaceControl API and is implemented by calling into SurfaceControl.
To deal with Java's lack of destructors and delayed garbage collection, we used to duplicate Surface.cpp objects in some case; this caused some issues because Surface is supposed to be reference-counted and unique.
