-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Support changing the mouse pointer #37882
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 13 commits
ec21312
f7591dc
b95a989
1accf8e
2bc371c
78137bd
a8d67b7
f5a4102
b7f883c
7b5b354
38ff429
783cbf6
894920c
8e7e62b
d5b98d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -51,6 +51,7 @@ function TdpClientCanvas(props: Props) { | |||||
| canvasOnMouseWheelScroll, | ||||||
| canvasOnContextMenu, | ||||||
| style, | ||||||
| updatePointer, | ||||||
| } = props; | ||||||
| const canvasRef = useRef<HTMLCanvasElement>(null); | ||||||
|
|
||||||
|
|
@@ -98,6 +99,42 @@ function TdpClientCanvas(props: Props) { | |||||
| } | ||||||
| }, [client, clientOnPngFrame]); | ||||||
|
|
||||||
| const previousCursor = useRef('auto'); | ||||||
|
|
||||||
| useEffect(() => { | ||||||
| if (client && updatePointer) { | ||||||
| const canvas = canvasRef.current; | ||||||
| const updatePointer = (pointer: { | ||||||
| data: ImageData | boolean; | ||||||
| hotspot_x?: number; | ||||||
| hotspot_y?: number; | ||||||
| }) => { | ||||||
| if (typeof pointer.data === 'boolean') { | ||||||
| if (pointer.data) { | ||||||
| canvas.style.cursor = previousCursor.current; | ||||||
| } else { | ||||||
| previousCursor.current = canvas.style.cursor; | ||||||
| canvas.style.cursor = 'none'; | ||||||
| } | ||||||
| return; | ||||||
| } | ||||||
| const cursor = document.createElement('canvas'); | ||||||
| cursor.width = pointer.data.width; | ||||||
| cursor.height = pointer.data.height; | ||||||
| cursor.getContext('2d').putImageData(pointer.data, 0, 0); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This probably doesn't change much in this case, but cursor
.getContext('2d', { colorSpace: pointer.data.colorSpace })
.putImageData(pointer.data, 0, 0); |
||||||
| canvas.style.cursor = `url(${cursor.toDataURL()}) ${ | ||||||
| pointer.hotspot_x | ||||||
| } ${pointer.hotspot_y}, auto`; | ||||||
| }; | ||||||
|
|
||||||
| client.on(TdpClientEvent.POINTER, updatePointer); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: for consistency, I'd use |
||||||
|
|
||||||
| return () => { | ||||||
| client.removeListener(TdpClientEvent.POINTER, updatePointer); | ||||||
| }; | ||||||
| } | ||||||
| }, [client, updatePointer]); | ||||||
|
|
||||||
| useEffect(() => { | ||||||
| if (client && clientOnBmpFrame) { | ||||||
| const canvas = canvasRef.current; | ||||||
|
|
@@ -401,6 +438,7 @@ export type Props = { | |||||
| canvasOnMouseWheelScroll?: (cli: TdpClient, e: WheelEvent) => void; | ||||||
| canvasOnContextMenu?: () => boolean; | ||||||
| style?: CSSProperties; | ||||||
| updatePointer: boolean; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can make this property optional, you won't have to pass
Suggested change
|
||||||
| }; | ||||||
|
|
||||||
| export default memo(TdpClientCanvas); | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -162,7 +162,7 @@ impl FastPathProcessor { | |||||||||||||||
| user_channel_id, | ||||||||||||||||
| // These should be set to the same values as they're set to in the | ||||||||||||||||
| // `Config` object in lib/srv/desktop/rdp/rdpclient/src/client.rs. | ||||||||||||||||
| no_server_pointer: true, | ||||||||||||||||
| no_server_pointer: false, | ||||||||||||||||
| pointer_software_rendering: false, | ||||||||||||||||
|
zmb3 marked this conversation as resolved.
|
||||||||||||||||
| } | ||||||||||||||||
| .build(), | ||||||||||||||||
|
|
@@ -184,6 +184,7 @@ impl FastPathProcessor { | |||||||||||||||
| cb_context: &JsValue, | ||||||||||||||||
| draw_cb: &js_sys::Function, | ||||||||||||||||
| respond_cb: &js_sys::Function, | ||||||||||||||||
| update_pointer_cb: &js_sys::Function, | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's document the signature of this function like we do for the others teleport/web/packages/teleport/src/ironrdp/src/lib.rs Lines 174 to 180 in 894920c
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might also be worth documenting the meanings of the different options for the |
||||||||||||||||
| ) -> Result<(), JsValue> { | ||||||||||||||||
| self.check_remote_fx(tdp_fast_path_frame)?; | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -211,13 +212,19 @@ impl FastPathProcessor { | |||||||||||||||
| UpdateKind::Region(region) => { | ||||||||||||||||
| outputs.push(ActiveStageOutput::GraphicsUpdate(region)); | ||||||||||||||||
| } | ||||||||||||||||
| UpdateKind::PointerDefault | ||||||||||||||||
| | UpdateKind::PointerHidden | ||||||||||||||||
| | UpdateKind::PointerPosition { .. } | ||||||||||||||||
| | UpdateKind::PointerBitmap(_) => { | ||||||||||||||||
| warn!("Pointer updates are not supported"); | ||||||||||||||||
| UpdateKind::PointerDefault => { | ||||||||||||||||
| outputs.push(ActiveStageOutput::PointerDefault); | ||||||||||||||||
| } | ||||||||||||||||
| UpdateKind::PointerHidden => { | ||||||||||||||||
| outputs.push(ActiveStageOutput::PointerHidden); | ||||||||||||||||
| } | ||||||||||||||||
| UpdateKind::PointerPosition { .. } => { | ||||||||||||||||
| warn!("Pointer position updates are not supported"); | ||||||||||||||||
| continue; | ||||||||||||||||
| } | ||||||||||||||||
|
probakowski marked this conversation as resolved.
|
||||||||||||||||
| UpdateKind::PointerBitmap(pointer) => { | ||||||||||||||||
| outputs.push(ActiveStageOutput::PointerBitmap(pointer)) | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -240,6 +247,30 @@ impl FastPathProcessor { | |||||||||||||||
| ActiveStageOutput::Terminate(_) => { | ||||||||||||||||
| return Err(JsValue::from_str("Terminate should never be returned")); | ||||||||||||||||
| } | ||||||||||||||||
| ActiveStageOutput::PointerBitmap(pointer) => { | ||||||||||||||||
| let data = &pointer.bitmap_data; | ||||||||||||||||
| let image_data = create_image_data_from_image_and_region( | ||||||||||||||||
| data, | ||||||||||||||||
| InclusiveRectangle { | ||||||||||||||||
| left: 0, | ||||||||||||||||
| top: 0, | ||||||||||||||||
| right: pointer.width - 1, | ||||||||||||||||
| bottom: pointer.height - 1, | ||||||||||||||||
| }, | ||||||||||||||||
| )?; | ||||||||||||||||
| update_pointer_cb.call3( | ||||||||||||||||
| cb_context, | ||||||||||||||||
| &JsValue::from(image_data), | ||||||||||||||||
| &JsValue::from(pointer.hotspot_x), | ||||||||||||||||
| &JsValue::from(pointer.hotspot_y), | ||||||||||||||||
| )?; | ||||||||||||||||
| } | ||||||||||||||||
| ActiveStageOutput::PointerDefault => { | ||||||||||||||||
| update_pointer_cb.call1(cb_context, &JsValue::from(true))?; | ||||||||||||||||
| } | ||||||||||||||||
| ActiveStageOutput::PointerHidden => { | ||||||||||||||||
| update_pointer_cb.call1(cb_context, &JsValue::from(false))?; | ||||||||||||||||
| } | ||||||||||||||||
| _ => { | ||||||||||||||||
| debug!("Unhandled ActiveStageOutput: {:?}", output); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add a short JSDoc comment explaining that if
dataisbooleanthentruemeans that we want to show a cursor andfalsemeans we want to hide it.