Code review comment for lp://staging/~justinmcp/oxide/cursor-support

Revision history for this message
Chris Coulson (chrisccoulson) wrote :

Thanks for working on this. This is pretty much ok. The only question I have is:

+void RenderWidgetHostView::UpdateCursor(const WebCursor& cursor) {
+ if (GetFormFactorHint() != FORM_FACTOR_DESKTOP) {
+ return; // Cursor only on desktop
+ }
+

Is this check really necessary? Is this done in other parts of the stack that process cursor changes? And do we know whether Chromium calls UpdateCursor() without mouse events?

+ QImage::Format format = QImage::Format_Invalid;
+ switch (cursor_info.custom_image.config()) {
+ case SkBitmap::kRGB_565_Config: format = QImage::Format_RGB16;
+ case SkBitmap::kARGB_4444_Config: format = QImage::Format_ARGB4444_Premultiplied;
+ case SkBitmap::kARGB_8888_Config: format = QImage::Format_ARGB32_Premultiplied;
+ default: ;
+ }

I was going to say that this bit is missing an indent for each of the case statements, but I've just noticed that this rule is already applied quite inconsistently in this file, so it doesn't matter for now :)

review: Needs Information

« Back to merge proposal