[jira] [Commented] (CB-9443) Windows camera picks incorrect maxResolution

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-9443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14654114#comment-14654114
 ] 

ASF GitHub Bot commented on CB-9443:


Github user purplecabbage commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-camera/pull/111#discussion_r36225312
  
--- Diff: src/windows/CameraProxy.js ---
@@ -696,21 +696,22 @@ function 
takePictureFromCameraWindows(successCallback, errorCallback, args) {
 // decide which max pixels should be supported by targetWidth or 
targetHeight.
 var maxRes = null;
 var UIMaxRes = WMCapture.CameraCaptureUIMaxPhotoResolution;
+var totalPixels = targetWidth * targetHeight;
 switch (true) {
-case (targetWidth >= 1280 || targetHeight >= 960) :
-cameraCaptureUI.photoSettings.maxResolution = UIMaxRes.large3M;
+case (totalPixels <= 320 * 240):
+cameraCaptureUI.photoSettings.maxResolution = 
UIMaxRes.verySmallQvga;
 break;
-case (targetWidth >= 1024 || targetHeight >= 768) :
-maxRes = UIMaxRes.mediumXga;
+case (totalPixels <= 640 * 480):
+maxRes = UIMaxRes.smallVga;
 break;
-case(targetWidth >= 800 || targetHeight >= 600) :
+case (totalPixels <= 1024 * 768):
 maxRes = UIMaxRes.mediumXga;
 break;
-case  (targetWidth >= 640 || targetHeight >= 480) :
-maxRes = UIMaxRes.smallVga;
+case (totalPixels <= 3 * 1000 * 1000):
+maxRes = UIMaxRes.large3M;
 break;
-case(targetWidth >= 320 || targetHeight >= 240) :
-maxRes = UIMaxRes.verySmallQvga;
+case (totalPixels <= 5 * 1000 * 1000):
+maxRes = UIMaxRes.veryLarge5M;
 break;
 default :
 maxRes = UIMaxRes.highestAvailable;
--- End diff --

Yes, the 'faster' argument is not particularly strong in this case
considering how often it will be called.



My team is hiring!
@purplecabbage
risingj.com

On Tue, Aug 4, 2015 at 8:08 AM, Rob Paveza  wrote:

> In src/windows/CameraProxy.js
> 

> :
>
> >  break;
> > -case(targetWidth >= 320 || targetHeight >= 240) :
> > -maxRes = UIMaxRes.verySmallQvga;
> > +case (totalPixels <= 5 * 1000 * 1000):
> > +maxRes = UIMaxRes.veryLarge5M;
> >  break;
> >  default :
> >  maxRes = UIMaxRes.highestAvailable;
>
> switch(true) would only be faster in the event that your cases were
> constants and you were doing explicit equality checks (e.g., case 10:).
> They're not, though, and because you're using greater-than comparisons, 
you
> lose the ability of the runtime compiler to optimize using jump-tables. If
> we're really concerned about perf, I'd suggest adding profiling code to
> order the most-common to the least-common cases; but I think this code 
will
> run so infrequently, it's not a perf-critical path.
>
> —
> Reply to this email directly or view it on GitHub
> 
> .
>



> Windows camera picks incorrect maxResolution
> 
>
> Key: CB-9443
> URL: https://issues.apache.org/jira/browse/CB-9443
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: Plugin Camera
> Environment: Windows Desktop, Windows Phone 10
>Reporter: Murat Sutunc
>Assignee: Murat Sutunc
>  Labels: Windows
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-9443) Windows camera picks incorrect maxResolution

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-9443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14653781#comment-14653781
 ] 

ASF GitHub Bot commented on CB-9443:


Github user robpaveza commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-camera/pull/111#discussion_r36199186
  
--- Diff: src/windows/CameraProxy.js ---
@@ -696,21 +696,22 @@ function 
takePictureFromCameraWindows(successCallback, errorCallback, args) {
 // decide which max pixels should be supported by targetWidth or 
targetHeight.
 var maxRes = null;
 var UIMaxRes = WMCapture.CameraCaptureUIMaxPhotoResolution;
+var totalPixels = targetWidth * targetHeight;
 switch (true) {
-case (targetWidth >= 1280 || targetHeight >= 960) :
-cameraCaptureUI.photoSettings.maxResolution = UIMaxRes.large3M;
+case (totalPixels <= 320 * 240):
+cameraCaptureUI.photoSettings.maxResolution = 
UIMaxRes.verySmallQvga;
 break;
-case (targetWidth >= 1024 || targetHeight >= 768) :
-maxRes = UIMaxRes.mediumXga;
+case (totalPixels <= 640 * 480):
+maxRes = UIMaxRes.smallVga;
 break;
-case(targetWidth >= 800 || targetHeight >= 600) :
+case (totalPixels <= 1024 * 768):
 maxRes = UIMaxRes.mediumXga;
 break;
-case  (targetWidth >= 640 || targetHeight >= 480) :
-maxRes = UIMaxRes.smallVga;
+case (totalPixels <= 3 * 1000 * 1000):
+maxRes = UIMaxRes.large3M;
 break;
-case(targetWidth >= 320 || targetHeight >= 240) :
-maxRes = UIMaxRes.verySmallQvga;
+case (totalPixels <= 5 * 1000 * 1000):
+maxRes = UIMaxRes.veryLarge5M;
 break;
 default :
 maxRes = UIMaxRes.highestAvailable;
--- End diff --

switch(true) would only be faster in the event that your cases were 
constants and you were doing explicit equality checks (e.g., `case 10:`).  
They're not, though, and because you're using greater-than comparisons, you 
lose the ability of the runtime compiler to optimize using jump-tables.  If 
we're really concerned about perf, I'd suggest adding profiling code to order 
the most-common to the least-common cases; but I think this code will run so 
infrequently, it's not a perf-critical path.


> Windows camera picks incorrect maxResolution
> 
>
> Key: CB-9443
> URL: https://issues.apache.org/jira/browse/CB-9443
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: Plugin Camera
> Environment: Windows Desktop, Windows Phone 10
>Reporter: Murat Sutunc
>Assignee: Murat Sutunc
>  Labels: Windows
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-9443) Windows camera picks incorrect maxResolution

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-9443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14652712#comment-14652712
 ] 

ASF GitHub Bot commented on CB-9443:


Github user rgecy commented on the pull request:


https://github.com/apache/cordova-plugin-camera/pull/111#issuecomment-127426047
  
How do I unsubscribe from this plugin list. I do not want to get emails on
every update.
On Aug 3, 2015 6:48 PM, "asfgit"  wrote:

> Closed #111 
> via ee5cfe8
> 

> .
>
> —
> Reply to this email directly or view it on GitHub
> 
> .
>



> Windows camera picks incorrect maxResolution
> 
>
> Key: CB-9443
> URL: https://issues.apache.org/jira/browse/CB-9443
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: Plugin Camera
> Environment: Windows Desktop, Windows Phone 10
>Reporter: Murat Sutunc
>Assignee: Murat Sutunc
>  Labels: Windows
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-9443) Windows camera picks incorrect maxResolution

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-9443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14652706#comment-14652706
 ] 

ASF subversion and git services commented on CB-9443:
-

Commit ee5cfe89a594ba8a86ca36fe67e7a040cc3d5e17 in cordova-plugin-camera's 
branch refs/heads/master from [~muratsu]
[ https://git-wip-us.apache.org/repos/asf?p=cordova-plugin-camera.git;h=ee5cfe8 
]

CB-9443 Pick correct maxResolution
This closes #111, closes #56


> Windows camera picks incorrect maxResolution
> 
>
> Key: CB-9443
> URL: https://issues.apache.org/jira/browse/CB-9443
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: Plugin Camera
> Environment: Windows Desktop, Windows Phone 10
>Reporter: Murat Sutunc
>Assignee: Murat Sutunc
>  Labels: Windows
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-9443) Windows camera picks incorrect maxResolution

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-9443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14652707#comment-14652707
 ] 

ASF GitHub Bot commented on CB-9443:


Github user asfgit closed the pull request at:

https://github.com/apache/cordova-plugin-camera/pull/111


> Windows camera picks incorrect maxResolution
> 
>
> Key: CB-9443
> URL: https://issues.apache.org/jira/browse/CB-9443
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: Plugin Camera
> Environment: Windows Desktop, Windows Phone 10
>Reporter: Murat Sutunc
>Assignee: Murat Sutunc
>  Labels: Windows
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-9443) Windows camera picks incorrect maxResolution

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-9443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14652543#comment-14652543
 ] 

ASF GitHub Bot commented on CB-9443:


Github user muratsu commented on the pull request:


https://github.com/apache/cordova-plugin-camera/pull/111#issuecomment-127409849
  
Sorry didn't see your comment earlier. 
In terms of perf, I think this the usage of `if/else` vs `switch` is not 
really important here. Since we're only executing it once, the perf difference 
is not going to be noticable either case.
Readability is a bit more subjective - I don't have a preference in this 
case. I tend to use switches only when I have fall-through cases. 
I wasn't familiar with `switch(true)` pattern. It seemed like an 
anti-pattern to me at first but looks like it is a common pattern in dynamic 
languages. I'll revert it back to switch.


> Windows camera picks incorrect maxResolution
> 
>
> Key: CB-9443
> URL: https://issues.apache.org/jira/browse/CB-9443
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: Plugin Camera
> Environment: Windows Desktop, Windows Phone 10
>Reporter: Murat Sutunc
>Assignee: Murat Sutunc
>  Labels: Windows
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-9443) Windows camera picks incorrect maxResolution

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-9443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14652510#comment-14652510
 ] 

ASF GitHub Bot commented on CB-9443:


Github user purplecabbage commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-camera/pull/111#discussion_r36130954
  
--- Diff: src/windows/CameraProxy.js ---
@@ -696,21 +696,22 @@ function 
takePictureFromCameraWindows(successCallback, errorCallback, args) {
 // decide which max pixels should be supported by targetWidth or 
targetHeight.
 var maxRes = null;
 var UIMaxRes = WMCapture.CameraCaptureUIMaxPhotoResolution;
+var totalPixels = targetWidth * targetHeight;
 switch (true) {
-case (targetWidth >= 1280 || targetHeight >= 960) :
-cameraCaptureUI.photoSettings.maxResolution = UIMaxRes.large3M;
+case (totalPixels <= 320 * 240):
+cameraCaptureUI.photoSettings.maxResolution = 
UIMaxRes.verySmallQvga;
 break;
-case (targetWidth >= 1024 || targetHeight >= 768) :
-maxRes = UIMaxRes.mediumXga;
+case (totalPixels <= 640 * 480):
+maxRes = UIMaxRes.smallVga;
 break;
-case(targetWidth >= 800 || targetHeight >= 600) :
+case (totalPixels <= 1024 * 768):
 maxRes = UIMaxRes.mediumXga;
 break;
-case  (targetWidth >= 640 || targetHeight >= 480) :
-maxRes = UIMaxRes.smallVga;
+case (totalPixels <= 3 * 1000 * 1000):
+maxRes = UIMaxRes.large3M;
 break;
-case(targetWidth >= 320 || targetHeight >= 240) :
-maxRes = UIMaxRes.verySmallQvga;
+case (totalPixels <= 5 * 1000 * 1000):
+maxRes = UIMaxRes.veryLarge5M;
 break;
 default :
 maxRes = UIMaxRes.highestAvailable;
--- End diff --

switch(true) is faster, + Personally, I find it easier to read.


> Windows camera picks incorrect maxResolution
> 
>
> Key: CB-9443
> URL: https://issues.apache.org/jira/browse/CB-9443
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: Plugin Camera
> Environment: Windows Desktop, Windows Phone 10
>Reporter: Murat Sutunc
>Assignee: Murat Sutunc
>  Labels: Windows
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-9443) Windows camera picks incorrect maxResolution

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-9443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14652502#comment-14652502
 ] 

ASF GitHub Bot commented on CB-9443:


Github user robpaveza commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-camera/pull/111#discussion_r36130435
  
--- Diff: src/windows/CameraProxy.js ---
@@ -696,21 +696,22 @@ function 
takePictureFromCameraWindows(successCallback, errorCallback, args) {
 // decide which max pixels should be supported by targetWidth or 
targetHeight.
 var maxRes = null;
 var UIMaxRes = WMCapture.CameraCaptureUIMaxPhotoResolution;
+var totalPixels = targetWidth * targetHeight;
 switch (true) {
-case (targetWidth >= 1280 || targetHeight >= 960) :
-cameraCaptureUI.photoSettings.maxResolution = UIMaxRes.large3M;
+case (totalPixels <= 320 * 240):
+cameraCaptureUI.photoSettings.maxResolution = 
UIMaxRes.verySmallQvga;
 break;
-case (targetWidth >= 1024 || targetHeight >= 768) :
-maxRes = UIMaxRes.mediumXga;
+case (totalPixels <= 640 * 480):
+maxRes = UIMaxRes.smallVga;
 break;
-case(targetWidth >= 800 || targetHeight >= 600) :
+case (totalPixels <= 1024 * 768):
 maxRes = UIMaxRes.mediumXga;
 break;
-case  (targetWidth >= 640 || targetHeight >= 480) :
-maxRes = UIMaxRes.smallVga;
+case (totalPixels <= 3 * 1000 * 1000):
+maxRes = UIMaxRes.large3M;
 break;
-case(targetWidth >= 320 || targetHeight >= 240) :
-maxRes = UIMaxRes.verySmallQvga;
+case (totalPixels <= 5 * 1000 * 1000):
+maxRes = UIMaxRes.veryLarge5M;
 break;
 default :
 maxRes = UIMaxRes.highestAvailable;
--- End diff --

Add `break;` after this line.


> Windows camera picks incorrect maxResolution
> 
>
> Key: CB-9443
> URL: https://issues.apache.org/jira/browse/CB-9443
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: Plugin Camera
> Environment: Windows Desktop, Windows Phone 10
>Reporter: Murat Sutunc
>Assignee: Murat Sutunc
>  Labels: Windows
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-9443) Windows camera picks incorrect maxResolution

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-9443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14652500#comment-14652500
 ] 

ASF GitHub Bot commented on CB-9443:


Github user muratsu commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-camera/pull/111#discussion_r36130273
  
--- Diff: src/windows/CameraProxy.js ---
@@ -696,21 +696,22 @@ function 
takePictureFromCameraWindows(successCallback, errorCallback, args) {
 // decide which max pixels should be supported by targetWidth or 
targetHeight.
 var maxRes = null;
 var UIMaxRes = WMCapture.CameraCaptureUIMaxPhotoResolution;
+var totalPixels = targetWidth * targetHeight;
 switch (true) {
--- End diff --

didn't notice `switch (true)` was just correcting the case params but wow - 
haha. definetely turning this to a `if/else`


> Windows camera picks incorrect maxResolution
> 
>
> Key: CB-9443
> URL: https://issues.apache.org/jira/browse/CB-9443
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: Plugin Camera
> Environment: Windows Desktop, Windows Phone 10
>Reporter: Murat Sutunc
>Assignee: Murat Sutunc
>  Labels: Windows
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-9443) Windows camera picks incorrect maxResolution

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-9443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14652495#comment-14652495
 ] 

ASF GitHub Bot commented on CB-9443:


Github user robpaveza commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-camera/pull/111#discussion_r36129853
  
--- Diff: src/windows/CameraProxy.js ---
@@ -696,21 +696,22 @@ function 
takePictureFromCameraWindows(successCallback, errorCallback, args) {
 // decide which max pixels should be supported by targetWidth or 
targetHeight.
 var maxRes = null;
 var UIMaxRes = WMCapture.CameraCaptureUIMaxPhotoResolution;
+var totalPixels = targetWidth * targetHeight;
 switch (true) {
--- End diff --

As long as you're refactoring the numbers, let's also turn this from 
`switch (true)` into `if`/`else`.


> Windows camera picks incorrect maxResolution
> 
>
> Key: CB-9443
> URL: https://issues.apache.org/jira/browse/CB-9443
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: Plugin Camera
> Environment: Windows Desktop, Windows Phone 10
>Reporter: Murat Sutunc
>Assignee: Murat Sutunc
>  Labels: Windows
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-9443) Windows camera picks incorrect maxResolution

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-9443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14652389#comment-14652389
 ] 

ASF GitHub Bot commented on CB-9443:


GitHub user muratsu opened a pull request:

https://github.com/apache/cordova-plugin-camera/pull/111

CB-9443 Pick correct maxResolution

According to MSDN docs we're currently not picking the correct 
maxResolution for images:

https://msdn.microsoft.com/en-us/library/windows/apps/windows.media.capture.cameracaptureuimaxphotoresolution.aspx

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/MSOpenTech/cordova-plugin-camera CB-9443

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-plugin-camera/pull/111.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #111


commit f60324e71bc0a894d91d83387533b2c274c1b908
Author: Murat Sutunc 
Date:   2015-08-03T19:57:58Z

CB-9443 Pick correct maxResolution




> Windows camera picks incorrect maxResolution
> 
>
> Key: CB-9443
> URL: https://issues.apache.org/jira/browse/CB-9443
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: Plugin Camera
> Environment: Windows Desktop, Windows Phone 10
>Reporter: Murat Sutunc
>Assignee: Murat Sutunc
>  Labels: Windows
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org